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

ceph-fuse: link to libfuse3 and pass -o big_writes to libfuse if libfuse < 3.0.0 #34531

Merged
merged 4 commits into from Apr 22, 2020
Merged

ceph-fuse: link to libfuse3 and pass -o big_writes to libfuse if libfuse < 3.0.0 #34531

merged 4 commits into from Apr 22, 2020

Conversation

lxbsz
Copy link
Member

@lxbsz lxbsz commented Apr 13, 2020

The big_writes is deprecated since libfuse 3.0. But if ceph-fuse is
linked to libfuse < 3.0, without big_writes option enabled, the write
performance is very poor.

Fixes: https://tracker.ceph.com/issues/44885
Signed-off-by: Xiubo Li xiubli@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@lxbsz lxbsz requested review from tchaikov and ukernel April 13, 2020 08:59
@lxbsz lxbsz force-pushed the libfuse3 branch 2 times, most recently from 2fc6762 to f0febc5 Compare April 13, 2020 09:43
ukernel
ukernel reviewed Apr 13, 2020
View reviewed changes
src/common/options.cc Outdated Show resolved Hide resolved
@gregsfortytwo
Copy link
Member

Please remember to add the right labels to your PRs so they filter correctly! :)

@lxbsz
Copy link
Member Author

lxbsz commented Apr 14, 2020

Please remember to add the right labels to your PRs so they filter correctly! :)

Okay, before It seemed I had no permission to set it, but it works now. Will do that for the new ones in future.

@tchaikov tchaikov changed the title ceph-fuse: enable the big writes with libfuse < 3.0.0 ceph-fuse: pass -o big_writes to libfuse if libfuse < 3.0.0 Apr 14, 2020
@lxbsz lxbsz changed the title ceph-fuse: pass -o big_writes to libfuse if libfuse < 3.0.0 ceph-fuse: link to libfuse3 and pass -o big_writes to libfuse if libfuse < 3.0.0 Apr 16, 2020
@lxbsz
Copy link
Member Author

lxbsz commented Apr 16, 2020

@ukernel @tchaikov
This has confilict with supporting to the libfuse3 one, I added the linking to libfuse3 here.

@gregsfortytwo
I removed the wip-grep-testing.
Thanks

@lxbsz lxbsz force-pushed the libfuse3 branch 2 times, most recently from 19d1649 to bfda0bd Compare April 16, 2020 13:21
ukernel
ukernel reviewed Apr 17, 2020
View reviewed changes
src/os/FuseStore.cc Outdated Show resolved Hide resolved
@ukernel
Copy link
Contributor

ukernel commented Apr 17, 2020

also see #34604

@ukernel ukernel closed this Apr 17, 2020
@lxbsz
Copy link
Member Author

lxbsz commented Apr 17, 2020

also see #34604

@ukernel The #34604 will only handle the Cmake stuff, and we still need this one.

@ukernel ukernel reopened this Apr 17, 2020
@tchaikov
Copy link
Contributor

#34604 has been merged.

@gregsfortytwo
Copy link
Member

Well the earlier version of this passed testing, will pull it in to the next one again. :)

@gregsfortytwo
Copy link
Member

@lxbsz I assume the force-push was just to rebase, no code changes that would invalidate the existing reviews?

@gregsfortytwo
Copy link
Member

@lxbsz while it passed the Jenkins "make check", pulling it into an integration branch resulted in build failures on Bionic
https://shaman.ceph.com/builds/ceph/wip-greg-testing-417-2/7e58a389efd0c8f4a8ce5e2661acbca8f2f712db/default/202328/

/build/ceph-16.0.0-896-g7e58a38/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported

Skimming over things, I assume it's because os_truncate now takes a more complicated type in libfuse3.

@lxbsz
ceph-fuse: enable the big writes as default
1c3dd60 
When the libfuse version < 3, without the big_writes options enabled
the write perf is very poor.

Fixes: https://tracker.ceph.com/issues/44885
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2020

@lxbsz I assume the force-push was just to rebase, no code changes that would invalidate the existing reviews?

Yeah, no confilict will happen with @tchaikov 's PR.

@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2020 •
edited

@lxbsz while it passed the Jenkins "make check", pulling it into an integration branch resulted in build failures on Bionic
https://shaman.ceph.com/builds/ceph/wip-greg-testing-417-2/7e58a389efd0c8f4a8ce5e2661acbca8f2f712db/default/202328/

/build/ceph-16.0.0-896-g7e58a38/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported

Skimming over things, I assume it's because os_truncate now takes a more complicated type in libfuse3.

Yeah, in libfuse3 the options has changed much and are split into 2 groups, one of the conn and one for mounting as usual, I will spend more time on the libfuse3 and check them in detail, this PR will do the basic support the libfuse3.

@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2020

@lxbsz while it passed the Jenkins "make check", pulling it into an integration branch resulted in build failures on Bionic
https://shaman.ceph.com/builds/ceph/wip-greg-testing-417-2/7e58a389efd0c8f4a8ce5e2661acbca8f2f712db/default/202328/

/build/ceph-16.0.0-896-g7e58a38/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported

Skimming over things, I assume it's because os_truncate now takes a more complicated type in libfuse3.

Yeah, in libfuse3 the options has changed much and are split into 2 groups, one of the conn and one for mounting as usual, I will spend more time on the libfuse3 and check them in detail, this PR will do the basic support the libfuse3.

This dues to some members are removed in linfuse3, and I jsut delete them if not defined, will fix it.

/build/ceph-16.0.0-874-g213214f/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported
 };
 ^

/build/ceph-16.0.0-874-g213214f/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported
/build/ceph-16.0.0-874-g213214f/src/os/FuseStore.cc:1179:1: sorry, unimplemented: non-trivial designated initializers not supported

@lxbsz
rbd-fuse: switch to use utimens instead
d5999a9 
The utime is deprecated.

Fixes: https://tracker.ceph.com/issues/44891
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
rbd-fuse: remove FUSE_CAP_BIG_WRITES when linking to libfuse3
32a45b6 
The big-writes is enabled as default from 3.0.

Fixes: https://tracker.ceph.com/issues/44891
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
ceph-fuse: link to libfuse3 if fuse3 is installed
cb0a600 
Fixes: https://tracker.ceph.com/issues/44891
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2020

@gregsfortytwo It's okay now.

https://shaman.ceph.com/builds/ceph/wip-lxb-libfuse3-2020-04-18-2/c96019781b7a94b2e18c4cae841fb1205f7df106/default/202451/

@ukernel
Copy link
Contributor

ukernel commented Apr 21, 2020

LGTM

@lxbsz
Copy link
Member Author

lxbsz commented Apr 21, 2020

The integration test:
http://pulpito.ceph.com/xiubli-2020-04-21_03:10:12-fs:basic_functional-wip-lxb-libfuse3-2020-04-18-2-distro-basic-smithi/

@gregsfortytwo
Copy link
Member

http://pulpito.front.sepia.ceph.com/gregf-2020-04-21_21:12:13-fs-wip-greg-testing-421-1-distro-basic-smithi/ looks good; few infra bugs and known issues

@gregsfortytwo gregsfortytwo merged commit ce0845c into ceph:master Apr 22, 2020
This was referenced Apr 27, 2020
Merged
Merged
@gman0 gman0 mentioned this pull request Mar 19, 2021
Closed
@Rakshith-R Rakshith-R mentioned this pull request Jan 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@ukernel ukernel ukernel approved these changes

@gregsfortytwo gregsfortytwo Awaiting requested review from gregsfortytwo

Assignees

@lxbsz lxbsz

Labels
cephfs Ceph File System needs-qa needs-review wip-greg-testing
Projects
None yet
Milestone
No milestone
4 participants
@lxbsz @gregsfortytwo @ukernel @tchaikov