On Wed, 24 Jan 2024 at 15:11, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Wed, Jan 24, 2024 at 2:17 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > On Wed, 24 Jan 2024 at 12:30, Jingbo Xu <jefflexu@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx> > > > --- > > > RFC: https://lore.kernel.org/all/20240123093701.94166-1-jefflexu@xxxxxxxxxxxxxxxxx/ > > > --- > > > 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). > > Not that I really care what the name is, but overlayfs already has > ovl_export_fid_operations and the name aspires from AT_HANDLE_FID, > which is already documented in man pages. > > How about fuse_export_fid_operations? Okay, let's be consistent with overlayfs naming. Thanks, Miklos
