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

Feature: store handler objects instead of fh #6

Open
amosonn opened this issue Mar 17, 2017 · 3 comments
Open

Feature: store handler objects instead of fh #6

amosonn opened this issue Mar 17, 2017 · 3 comments

Comments

@amosonn
Copy link
Contributor

amosonn commented Mar 17, 2017

I think a pretty common pattern for both the open and the opendir methods would be to store some handler object, returning an fh as a key to access it later.

Assuming this, the library could request the higher-level FS to return an object from open and opendir, handler the storing of it themselves, handing to all calls which currently get fh (read, readir, etc.), and disposing of it on release and releasedir (without exposing them to the higher-level FS; if the implementer of that wants, they can add logic in drop).

I'm not sure if this belongs here; while this is not limiting in any sense (one can always choose the types of these objects to be u64 to retain current behaviour or () if they're implementing stateless access; this does require some thought about release, though), it does incur some inevitable overhead (at least until Rust lets one write monomorphisations specializations, where a special implementation could be written for u64 and (). Edit: rust-lang/rust#31844). I'm currently writing this logic as a separate crate, over fuse-mt - not yet in cargo, but you can see the code here: https://github.com/amosonn/rust_fuse_fl.

However, adding this to the lower level library will definitely have some benefits. Aside from removing another level of indirection for the unaffected methods (not all that important, I assume the compiler can optimize it away anyway), currently I have to use a RwLock over the entire HashMap to have interior mutability, and release calls actually block until all reads, etc. have completed (which is even worse, given that currently release isn't run in a separate thread). If this were part of fuse-mt, we could use a HashMap over Arcs instead, and release will simply remove the copy residing in the HashMap, so the drop will be called in the last thread using that file.

What do you think?

@amosonn
Copy link
Contributor Author

amosonn commented Apr 3, 2017

I've fleshed out most of what I had in mind there, have to think a bit what's the right way to test, but you can still get an impression of what it's like.
Do tell if you think this is a good idea, if so I would gladly craft this as a pull-request; apart from the benefits highlighted above, this would get rid of the awkward transmute I have to do right now in order to get the reference of the object through the RwLock to have a sane lifetime.

@amosonn
Copy link
Contributor Author

amosonn commented Apr 3, 2017

BTW, the Result<T> = result::Result<T, libc::c_int> could probably find a happy home here anyway.

@wfraser
Copy link
Owner

wfraser commented May 24, 2017

I'm not sure I like this idea. It basically entails having fuse-mt hold on to the object instead of the filesystem, and for the (presumably) common case where it's just a file descriptor or other ID, it's a waste of space until Rust gets specializations, and I don't see that happening any time soon. This is something that's likely to be implemented differently in different filesystems, and so it should live there.

As for the locking issue, moving it into fuse-mt isn't going to fix that. It would actually make it worse, because fuse-mt doesn't know how your filesystem is implemented, and so it would have to do that same locking all the time, for all calls that involve a fh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants