Received-SPF: pass (google.com: domain of linux-kernel+bounces-37116-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240124113042.44300-1-jefflexu@linux.alibaba.com>
 <CAJfpegtkSgRO-24bdnA4xUMFW5vFwSDQ7WkcowNR69zmbRwKqQ@mail.gmail.com> <CAOQ4uxjN6f=M8jjM0-_eysLy8Jx1+r0Dy3MhWHc8f2r7RnEmdQ@mail.gmail.com>
In-Reply-To: <CAOQ4uxjN6f=M8jjM0-_eysLy8Jx1+r0Dy3MhWHc8f2r7RnEmdQ@mail.gmail.com>
From: Miklos Szeredi <miklos@szeredi.hu>
Date: Wed, 24 Jan 2024 15:28:02 +0100
Message-ID: <CAJfpegtBod6ECv6O0GVXhLSGB5OH6P=uOpMN6JjAm3C+Mrg55g@mail.gmail.com>
Subject: Re: [PATCH] fuse: add support for explicit export disabling
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jingbo Xu <jefflexu@linux.alibaba.com>, linux-fsdevel@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, 24 Jan 2024 at 15:11, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Jan 24, 2024 at 2:17=E2=80=AFPM Miklos Szeredi <miklos@szeredi.hu=
> wrote:
> >
> > On Wed, 24 Jan 2024 at 12:30, Jingbo Xu <jefflexu@linux.alibaba.com> wr=
ote:
> > >
> > > open_by_handle_at(2) can fail with -ESTALE with a valid handle return=
ed
> > > 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=3Dnone" 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 s=
end
> > > a new FUSE_LOOKUP request upon inode cache miss since the previous in=
ode
> > > eviction.  Then the fuse daemon may fail the FUSE_LOOKUP request with
> > > -ENOENT as the cached metadata of the requested inode has already bee=
n
> > > 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, aft=
er
> > > which the application can fallback to open(2) to access files.
> > >
> > > Since this issue typically appears when entry_valid_timeout is 0 whic=
h
> > > 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 f=
or
> > > lookups of "." and "..", and there are existing fuse daemons supporti=
ng
> > > export without FUSE_EXPORT_SUPPORT set, for compatibility, we add a n=
ew
> > > 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@linu=
x.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 d=
entry *child)
> > >         return parent;
> > >  }
> > >
> > > +/* only for fid encoding; no support for file handle */
> > > +static const struct export_operations fuse_fid_operations =3D {
> >
> > 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