Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fusedev: support splice to handle FUSE requests. #116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fusedev: support splice to handle FUSE requests. #116

wants to merge 2 commits into from

Conversation

hhhhsdxxxx
Copy link

  1. Support enable splice_read/splice_write on FuseChannel.
  2. Add splice interface for ZeroCopyReader and ZeroCopyWriter.
  3. Add unit-test cases for splice interface.

@hhhhsdxxxx
Copy link
Author

It seems that async-io is not a complete implementation now, so splice only supports sync-io in this PR.

@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 5 times, most recently from 0d7f82b to 2dc412b Compare April 17, 2023 03:50
@hhhhsdxxxx
Copy link
Author

We test on network fs(based on fuse-backend-rs) using fio. (1 job, 128k bs, psync, randread)
For local socket: 10%~20% performance improvement.
For remote socket: 5%~10% performance improvement.

@hhhhsdxxxx
Copy link
Author

@bergwolf @jiangliu thanks for reviewing.

@hhhhsdxxxx
Copy link
Author

We also test on write cases:
For large user io (>128K), it has at least 10% improvement of performance.

@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 5 times, most recently from 140f5a8 to 6e3c298 Compare April 21, 2023 07:05
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 2 times, most recently from d6a2fd5 to 3264e60 Compare May 17, 2023 08:00
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 5 times, most recently from 5166af9 to 81ede27 Compare September 6, 2023 07:43
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 2 times, most recently from 7ec265d to f501838 Compare November 6, 2023 14:31
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 3 times, most recently from 2ecbefe to 6b3a95a Compare November 7, 2023 03:51
WeiZhang555
WeiZhang555 reviewed Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@WeiZhang555 WeiZhang555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that possible to add splice() usage to tests/passthrough as example, or as smoke CI?

src/api/filesystem/mod.rs Show resolved Hide resolved
src/transport/fusedev/linux_session.rs Show resolved Hide resolved
src/transport/mod.rs Outdated
if let ReaderInner::Pipe(p) = &mut self.inner {
p.splice_to(fd, count)
} else {
let mut file = unsafe { std::fs::File::from_raw_fd(f.as_raw_fd()) };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return an error here? Since only Pipe can be splice().

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea.
I have already done it.

src/transport/mod.rs Outdated Show resolved Hide resolved
src/transport/fusedev/linux_session.rs Show resolved Hide resolved
src/transport/fusedev/linux_session.rs
/// // after handle init request, we know whether kernel support splice read
/// if fs.is_support_splice_read() {
/// ch.enable_splice_read();
/// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If splice improves performance obviously, can we enable splice read/write automatically according to MyFs's FsOptions? In this way, we can still disable it by not passing SPLICE_READ/SPLICE_WRITE/SPLICE_MOVE.

Will splice bring problem if set as default?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splice improve performance of read/write operation.
but have overhead for those meta operations, for example lookup, getattr ...

src/transport/fusedev/linux_session.rs Outdated Show resolved Hide resolved
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 3 times, most recently from 7aa0491 to 44f56b6 Compare January 22, 2024 05:47
@hhhhsdxxxx
Copy link
Author

hhhhsdxxxx commented Jan 22, 2024 •
edited
Loading

Is that possible to add splice() usage to tests/passthrough as example, or as smoke CI?

I'll do it right now.

@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 6 times, most recently from 8646133 to 48edd73 Compare January 22, 2024 11:44
@hhhhsdxxxx
Copy link
Author

smoke test of splice is ready now.
xfstest take too much time.
so I manually trigger xfstest with splice enabled. Result is below:
https://github.com/cloud-hypervisor/fuse-backend-rs/actions/runs/7608075934/job/20716538551?pr=116

WeiZhang555
WeiZhang555 reviewed Jan 23, 2024
View reviewed changes
Copy link
Collaborator

@WeiZhang555 WeiZhang555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments, other parts LGTM

src/api/filesystem/mod.rs Show resolved Hide resolved
src/transport/fusedev/linux_session.rs Outdated Show resolved Hide resolved
src/api/server/sync_io.rs Show resolved Hide resolved
@hhhhsdxxxx hhhhsdxxxx force-pushed the henry.hj/splice branch 3 times, most recently from c2e1eb0 to 3687acc Compare February 28, 2024 12:25
henry.hj added 2 commits February 28, 2024 20:30
fusedev: support splice to handle FUSE requests.
6d5c717 
1. Support enable splice_read/splice_write on FuseChannel.
2. Add splice interface for ZeroCopyReader and ZeroCopyWriter.
3. Add unit-test cases for splice interface.

Signed-off-by: Henry Huang <henry.hj@antgroup.com>
passthroughfs: support splice for fusedev
ef49e30 
add a smoke testcase for splice

Signed-off-by: Henry Huang <henry.hj@antgroup.com>
WeiZhang555
WeiZhang555 approved these changes Feb 29, 2024
View reviewed changes
Copy link
Collaborator

@WeiZhang555 WeiZhang555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WeiZhang555 WeiZhang555 WeiZhang555 approved these changes

@bergwolf bergwolf Awaiting requested review from bergwolf bergwolf is a code owner

@imeoer imeoer Awaiting requested review from imeoer imeoer is a code owner

@jiangliu jiangliu Awaiting requested review from jiangliu

@liubogithub liubogithub Awaiting requested review from liubogithub liubogithub is a code owner

@eryugey eryugey Awaiting requested review from eryugey eryugey is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hhhhsdxxxx @WeiZhang555