Message-ID: <CAOQ4uxhOuBXT3tgoLxjh6efAwiOLg=oDxsyivLLMXCrSamSuEA@mail.gmail.com>
Date: Thu, 15 Jan 2026 16:31:55 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Chunsheng Luo <luochunsheng@...c.edu>
Cc: miklos@...redi.hu, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, paullawrence@...gle.com
Subject: Re: [RFC 2/2] fuse: Add new flag to reuse the backing file of fuse_inode

On Thu, Jan 15, 2026 at 3:35 PM Chunsheng Luo <luochunsheng@...c.edu> wrote:
>
>
>
> On 1/15/26 9:09 PM, Amir Goldstein wrote:
> > Hi Chunsheng,
> >
> > Please CC me for future fuse passthrough patch sets.
> >
> Ok.
>
> > On Thu, Jan 15, 2026 at 03:20:31PM +0800, Chunsheng Luo wrote:
> >> To simplify crash recovery and reduce performance impact, backing_ids
> >> are not persisted across daemon restarts. However, this creates a
> >> problem: when the daemon restarts and a process opens the same FUSE
> >> file, a new backing_id may be allocated for the same backing file. If
> >> the inode already has a cached backing file from before the restart,
> >> subsequent open requests with the new backing_id will fail in
> >> fuse_inode_uncached_io_start() due to fb mismatch, even though both
> >> IDs reference the identical underlying file.
> >
> > I don't think that your proposal makes this guaranty.
> >
>
> Yes, this proposal does not apply to all situations.
>
> >>
> >> Introduce the FOPEN_PASSTHROUGH_INODE_CACHE flag to address this
> >> issue. When set, the kernel reuses the backing file already cached in
> >> the inode.
> >>
> >> Signed-off-by: Chunsheng Luo <luochunsheng@...c.edu>
> >> ---
> >>   fs/fuse/iomode.c          |  2 +-
> >>   fs/fuse/passthrough.c     | 11 +++++++++++
> >>   include/uapi/linux/fuse.h |  2 ++
> >>   3 files changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> >> index 3728933188f3..b200bb248598 100644
> >> --- a/fs/fuse/iomode.c
> >> +++ b/fs/fuse/iomode.c
> >> @@ -163,7 +163,7 @@ static void fuse_file_uncached_io_release(struct fuse_file *ff,
> >>    */
> >>   #define FOPEN_PASSTHROUGH_MASK \
> >>      (FOPEN_PASSTHROUGH | FOPEN_DIRECT_IO | FOPEN_PARALLEL_DIRECT_WRITES | \
> >> -     FOPEN_NOFLUSH)
> >> +     FOPEN_NOFLUSH | FOPEN_PASSTHROUGH_INODE_CACHE)
> >>
> >>   static int fuse_file_passthrough_open(struct inode *inode, struct file *file)
> >>   {
> >> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> >> index 72de97c03d0e..fde4ac0c5737 100644
> >> --- a/fs/fuse/passthrough.c
> >> +++ b/fs/fuse/passthrough.c
> >> @@ -147,16 +147,26 @@ ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma)
> >>   /*
> >>    * Setup passthrough to a backing file.
> >>    *
> >> + * If fuse inode backing is provided and FOPEN_PASSTHROUGH_INODE_CACHE flag
> >> + * is set, try to reuse it first before looking up backing_id.
> >> + *
> >>    * Returns an fb object with elevated refcount to be stored in fuse inode.
> >>    */
> >>   struct fuse_backing *fuse_passthrough_open(struct file *file, int backing_id)
> >>   {
> >>      struct fuse_file *ff = file->private_data;
> >>      struct fuse_conn *fc = ff->fm->fc;
> >> +    struct fuse_inode *fi = get_fuse_inode(file->f_inode);
> >>      struct fuse_backing *fb = NULL;
> >>      struct file *backing_file;
> >>      int err;
> >>
> >> +    if (ff->open_flags & FOPEN_PASSTHROUGH_INODE_CACHE) {
> >> +            fb = fuse_backing_get(fuse_inode_backing(fi));
> >> +            if (fb)
> >> +                    goto do_open;
> >> +    }
> >> +
> >
> > Maybe an explicit FOPEN_PASSTHROUGH_INODE_CACHE flag is a good idea,
> > but just FYI, I intentionally reserved backing_id 0 for this purpose.
> > For example, for setting up the backing id on lookup [1] and then
> > open does not need to specify the backing_id.
> >
> > [1] https://lore.kernel.org/linux-fsdevel/20250804173228.1990317-1-paullawrence@google.com/
> >
>
> This is a great idea. However, we need to consider the lifecycle
> management of the backing file associated with a FUSE inode.
> Specifically, will the same backing_idbe retained for the entire
> lifetime of the FUSE inode until it is deleted?

It's not a good fit for servers that want to change the backing file
(like re-download). For these servers we have the existing file
open-to-close life cycle.

>
> Additionally, since each backing_idcorresponds to an open file
> descriptor (fd) for the backing file, if a fuse_inode holds onto a
> backing_id indefinitely without a suitable release mechanism, could this
> accumulation of file descriptors cause the process to exceed its open
> files limit?
>

There is no such accumulation.
fuse_inode refers to a single fuse_backing object.
fuse_file refers to a single fuse_backing object.
It can be the same (refcounted) object.

> > But what you are proposing is a little bit odd API IMO:
> > "Use this backing_id with this backing file, unless you find another
> >   backing file so use that one instead" - this sounds a bit awkward to me.
> >
> > I think it would be saner and simpler to relax the check in
> > fuse_inode_uncached_io_start() to check that old and new fuse_backing
> > objects refer to the same backing inode:
> >
> > diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> > index 3728933188f30..c6070c361d855 100644
> > --- a/fs/fuse/iomode.c
> > +++ b/fs/fuse/iomode.c
> > @@ -88,9 +88,9 @@ int fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing *fb)
> >       int err = 0;
> >
> >       spin_lock(&fi->lock);
> > -     /* deny conflicting backing files on same fuse inode */
> > +     /* deny conflicting backing inodes on same fuse inode */
> >       oldfb = fuse_inode_backing(fi);
> > -     if (fb && oldfb && oldfb != fb) {
> > +     if (fb && oldfb && file_inode(oldfb->file) != file_inode(fb->file)) {
> >               err = -EBUSY;
> >               goto unlock;
> >       }
> > --
> >
> > I don't think that this requires opt-in flag.
> >
> > Thanks,
> > Amir.
>
> I agree that modifying the condition to `file_inode(oldfb->file) !=
> file_inode(fb->file)` is a reasonable fix, and it does address the first
> scenario I described.
>
> However, it doesn't fully resolve the second scenario: in a read-only
> FUSE filesystem, the backing file itself might be cleaned up and
> re-downloaded (resulting in a new inode with identical content). In this
> case, reusing the cached fuse_inode's fb after a daemon restart still be
> safe, but the inode comparison would incorrectly reject it. Is there a
> more robust approach for handling this scenario?
>

There is a reason we added the restriction against associating
fuse file to different backing inodes.

mmap and reads from different files to the same inode need to be
cache coherent.

IOW, we intentionally do not support this setup without server restart
there is no reason for us to allow that after server restarts because
the consequense will be the same.

It does not sound like a good idea for the server to cleanup files
that are currently opened via fuse passthrough - is that something
that happens intentionally? after server restarts?

You could try to take a write lease to check if the file is currently
open for read/write to avoid cleanup in this case?

Thanks,
Amir.

Powered by blists - more mailing lists