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.

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

Make fuse_custom_io support multi-threaded event loop #927

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

legezywzh
Copy link
Contributor

Define a new clone_fd() helper for fuse_custom_io, users can implement their own clone fd logic.

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 16, 2024

There is a something similar like that in my uring branch (fuse-over-uring also needs fd-clone, but per queue).

Could you please change subject a bit? I don't see the direct relation of "multi-threaded event loop" to clone-fd. Maybe that is how your custom IO works, but I think that is an implementation details.
So maybe something like "Add clone-fd to custom IO". The commit message can then describe that you need that for multi-threading.

@legezywzh
Copy link
Contributor Author

There is a something similar like that in my uring branch (fuse-over-uring also needs fd-clone, but per queue).

io_uring is a great feature, hope it'll improve fuse filesystem's performance.

Could you please change subject a bit? I don't see the direct relation of "multi-threaded event loop" to clone-fd. Maybe that is how your custom IO works, but I think that is an implementation details. So maybe something like "Add clone-fd to custom IO". The commit message can then describe that you need that for multi-threading.

OK, thanks for this suggestion.

Define a new clone_fd() helper for fuse_custom_io, users
can implement their own clone fd logic.

Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
@bsbernd bsbernd merged commit 73cd124 into libfuse:master Apr 18, 2024
2 checks passed
@bsbernd
Copy link
Collaborator

bsbernd commented Apr 18, 2024

Thank you, merged.

@ashleypittman
Copy link
Collaborator

Sorry, I was going to comment on this one. fuse_session_custom_io doesn't have the same protection for adding entries to the end of structs that fuse_session_new does so extending this struct causes interoperability problems.

https://github.com/libfuse/libfuse/blob/master/lib/fuse_lowlevel.c#L3119

vs

libfuse/lib/fuse_lowlevel.c

Lines 3023 to 3034 in 73cd124

struct fuse_session *fuse_session_new(struct fuse_args *args,
const struct fuse_lowlevel_ops *op,
size_t op_size, void *userdata)
{
int err;
struct fuse_session *se;
struct mount_opts *mo;
if (sizeof(struct fuse_lowlevel_ops) < op_size) {
fuse_log(FUSE_LOG_ERR, "fuse: warning: library too old, some operations may not work\n");
op_size = sizeof(struct fuse_lowlevel_ops);
}

The clever bit is passing in op_size and using that as the memcpy so that any new members are blank. With this PR then using master against binaries built with older versions will now read a out-of-band value and use it for the new callback.

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 18, 2024

Ah oh right. Well, we cannot change fuse_session_new because that breaks everything and not only custom IO. Better would be if all these structs would be private and would need to be allocated as transparent object through libfuse. Given that custom IO is a rather new feature, I think we can switch it to that.

@ashleypittman
Copy link
Collaborator

I think the trick that fuse_session_new uses is a good one, what would be ideal would be if fuse_session_custom_io had used it from the start.

I was putting fuse_session_new forward as a way that this should be done. Perhaps we could use the symbol versioning to have old vs new versions of fuse_session_custom_io here?

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 18, 2024

It would break the API - the worst case. We can update fuse_session_custom_io

@ashleypittman
Copy link
Collaborator

It would break the API - the worst case. We can update fuse_session_custom_io

That's probably a good idea, at least as a start. It looks like this affects 3.13-3.16 only.

Do you want me to push a patch?

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 18, 2024

I'm just in the middle to type it

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 18, 2024

PR is here: #930
@legezywzh could you please also review? We also need to try to get other users of the custom-io path to review it.

@bsbernd
Copy link
Collaborator

bsbernd commented Apr 23, 2024

@legezywzh Please also see @ashleypittman patch to make it versioned #935. Actually I would like to get 3.17 with out ASAP - basically as soon as I'm done to finish putting the libfuse version into fuse_session_new.
I'm going to revert this patch here, if we don't have ABI/API compatibility ready for fuse_custom_io in time

@legezywzh
Copy link
Contributor Author

Sorry for late response, I'm away from libfuse work for days, I'll go through your discussions today.

bsbernd added a commit to bsbernd/libfuse that referenced this pull request May 22, 2024
* master:
  Enable passthrough mode for read/write operations (libfuse#919)
  Add in the libfuse version a program was compiled with (libfuse#942)
  Handle NO_OPEN/NO_OPENDIR support automatically (libfuse#949)
  Bump actions/checkout from 4.1.4 to 4.1.5 (libfuse#946)
  fuse_common.h: fix warning on _Static_assert() (libfuse#939)
  Fix missing fuse_loop_cfg_destroy() in fuse_session_loop_mt_31 (libfuse#944)
  Bump actions/checkout from 4.1.3 to 4.1.4 (libfuse#943)
  Use std=gnu11 (libfuse#940)
  Bump actions/checkout from 4.1.2 to 4.1.3 (libfuse#934)
  [libFuse 3.16.2]Compilation failure on freeBSD libfuse#936 (libfuse#938)
  Use single place to define the version and increase version to 3.17.0 (libfuse#932)
  example/: Convert all fuse_session_loop_mt users to 3.12 API (libfuse#931)
  passthrough_ll: fix fd leaks in lo_destroy() (libfuse#929)
  Add clone_fd to custom IO (libfuse#927)
  fix max_write update in do_init() (libfuse#926)
  fusermount: Fix use of uninitialized x_mnt_opts (libfuse#924)
  Add more documentation for FUSE_CAP_EXPORT_SUPPORT (libfuse#917)

	example/passthrough_hp.cc
	example/passthrough_ll.c
	include/fuse_kernel.h
	lib/fuse_i.h
	lib/fuse_lowlevel.c
	lib/fuse_versionscript
legezywzh pushed a commit to legezywzh/libfuse that referenced this pull request May 27, 2024
Fixes: 73cd124 ("Add clone_fd to custom IO (libfuse#927)")
Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
legezywzh pushed a commit to legezywzh/libfuse that referenced this pull request May 28, 2024
Fixes: 73cd124 ("Add clone_fd to custom IO (libfuse#927)")
Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
bsbernd pushed a commit that referenced this pull request Jun 1, 2024
Fixes: 73cd124 ("Add clone_fd to custom IO (#927)")

Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants