Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 24 Jan 2024 20:50:21 +0800 | Subject | Re: [PATCH] fuse: add support for explicit export disabling | From | Jingbo Xu <> |
| |
On 1/24/24 8:16 PM, Miklos Szeredi wrote: > On Wed, 24 Jan 2024 at 12:30, Jingbo Xu <jefflexu@linux.alibaba.com> wrote: >> >> open_by_handle_at(2) can fail with -ESTALE with a valid handle returned >> by a previous name_to_handle_at(2) for evicted fuse inodes, which is >> especially common when entry_valid_timeout is 0, e.g. when the fuse >> daemon is in "cache=none" mode. >> >> The time sequence is like: >> >> name_to_handle_at(2) # succeed >> evict fuse inode >> open_by_handle_at(2) # fail >> >> The root cause is that, with 0 entry_valid_timeout, the dput() called in >> name_to_handle_at(2) will trigger iput -> evict(), which will send >> FUSE_FORGET to the daemon. The following open_by_handle_at(2) will send >> a new FUSE_LOOKUP request upon inode cache miss since the previous inode >> eviction. Then the fuse daemon may fail the FUSE_LOOKUP request with >> -ENOENT as the cached metadata of the requested inode has already been >> cleaned up during the previous FUSE_FORGET. The returned -ENOENT is >> treated as -ESTALE when open_by_handle_at(2) returns. >> >> This confuses the application somehow, as open_by_handle_at(2) fails >> when the previous name_to_handle_at(2) succeeds. The returned errno is >> also confusing as the requested file is not deleted and already there. >> It is reasonable to fail name_to_handle_at(2) early in this case, after >> which the application can fallback to open(2) to access files. >> >> Since this issue typically appears when entry_valid_timeout is 0 which >> is configured by the fuse daemon, the fuse daemon is the right person to >> explicitly disable the export when required. >> >> Also considering FUSE_EXPORT_SUPPORT actually indicates the support for >> lookups of "." and "..", and there are existing fuse daemons supporting >> export without FUSE_EXPORT_SUPPORT set, for compatibility, we add a new >> INIT flag for such purpose. > > This looks good overall. > >> >> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> >> --- >> RFC: https://lore.kernel.org/all/20240123093701.94166-1-jefflexu@linux.alibaba.com/ >> --- >> fs/fuse/inode.c | 11 ++++++++++- >> include/uapi/linux/fuse.h | 2 ++ >> 2 files changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c >> index 2a6d44f91729..851940c0e930 100644 >> --- a/fs/fuse/inode.c >> +++ b/fs/fuse/inode.c >> @@ -1110,6 +1110,11 @@ static struct dentry *fuse_get_parent(struct dentry *child) >> return parent; >> } >> >> +/* only for fid encoding; no support for file handle */ >> +static const struct export_operations fuse_fid_operations = { > > Nit: I'd call this fuse_no_export_operations (or something else that > emphasizes the fact that this is only for encoding and not for full > export support).
OK I will rename it to fuse_no_export_operations.
By the way do I need to bump and update the minor version of FUSE protocol?
-- Thanks, Jingbo
| ![\](/images/icornerr.gif) |