Message-ID: <CAOQ4uxjX1Cs-UYEKZfNtCz_31JiH74KaC_EdU07oxX-nCcirFQ@mail.gmail.com>
Date: Mon, 14 Jul 2025 17:20:27 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Paul Lawrence <paullawrence@...gle.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, Bernd Schubert <bernd.schubert@...tmail.fm>
Subject: Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories

On Thu, Jun 19, 2025 at 10:29 PM Amir Goldstein <amir73il@...il.com> wrote:
>
> On Thu, Jun 19, 2025 at 9:50 PM Paul Lawrence <paullawrence@...gle.com> wrote:
> >
> > Hi Amir,
> >
> > Thank you for your detailed reply. My intent with this patch was to see if there
> > was interest (a definite yes) and to see what path would best get us
> > to our common
> > goal.
> >
> > I'm thinking the best approach is to start with your ops_mask API. In
> > fact, that solves
> > the biggest single problem with my future patch set, which was that it
> > was going to be
> > huge and not realistically divisible, since you need everything for
> > directory passthrough
> > to work without the mask. Your way allows us to proceed in nice
> > logical steps, which is
> > much, much better. Thank you for that suggestion.
> >
> > So my follow-up question is: What can I do to help get the
> > foundational patches you
> > wrote upstreamed?
>
> Well you can always take them and re-shape them and post them
> to see what the maintainers think and address the feedback.
> But I can try to beat them to shape myself to at least post v1.
>
> >
> > In the meantime, a few thoughts on your comments. (Note that one of
> > the beauties of
> > your suggestion is that we don't need to agree on any of this to get
> > started - we can
> > discuss them in detail when we get to the specific ops that require them.)
> >
> > 1) Yes, let's use backing_id. I won't mention that again.
> >
> > 2) The backing path per dentry comes from the way dentry_open works.
> > If we are going to
> > attach a file to a lookup, we have to put something into the
> > fuse_dentry or the fuse_inode.
>
> There is already fuse_backing *fb in fuse_inode.
> I don't understand why anything else is needed for implementing
> passthrough dir ops.
>
> > This makes more sense once you see points 3 & 4 below - without them,
> > we have an open
> > file, so why not just use it?
>
> We need to make the code simple enough.
> Not add things that are not needed.
>
> >
> > 3) A cute idea that we had that seems to work is to allow negative
> > dentries as backing
> > dentries. It appears to work well - for instance, a create first looks
> > up the (negative) dentry
> > then creates the file into that dentry. If the lookup puts a negative
> > dentry as the backing
> > file, we can now just use vfs_create to create the backing file.
> >
>
> That sounds like trouble.
> Overalyfs effectively implements passthrough dir ops.
> It doesn't keep negative backing dentries, so I doubt that this is needed.
>
> > This means that only FUSE_LOOKUP and (I think) FUSE_READDIRPLUS need to have
> > the ability to accept backing_ids. I think is is both more elegant
> > conceptually, simpler to
> > code in the kernel *and* simpler to use in the daemon.
> >
> > 4) Having to open a file for it to be passed into a lookup is
> > problematic. Imagine
> > readdirplus on a large folder. We would need to open every single
> > backing file, and it
> > would stay open until the dentry was removed from the cache.
>
> We are talking about opening a O_PATH fd at lookup.
> The daemon does not need to keep this O_PATH fd open,
> although production daemons today (e.g. virtiofsd) anyway
> keep an open O_PATH fd per fuse inode in cache.
>
> Maybe it is a problem, but I am not convinced that it is, so
> maybe I need more details about what problems this is causing.
> If you are going to pin the backing inode/dentry to cache, then most
> of the memory resources are already taken, the extra file does not add
> much memory and it is currently not accounted for in any process.
>
> >
> > Both of these suggest that rather than just passing a backing_id to FUSE_LOOKUP
> > and FUSE_READDIRPLUS we should be able to pass a backing_id and a relative path.
> > This is where the idea of putting the backing path into the fuse
> > dentry comes from.
> >
>
> Sorry this is too much hand waving.
> I still don't understand what problem attaching a backing path to every dentry
> solves. You will have to walk me through exactly what the problem is with
> having the backing file/path attached to the inode.
>
> > I don't *think* this creates any security issues, so long as the
> > relative path is traversed
> > in the context of the daemon. (We might want to ban '..' and traverses
> > over file systems.)
>
> Sorry you lost me.
> I do not understand the idea of backing_id and a relative path.
> passthrough of READDIRPLUS is complicated.
> If you have an idea I need to see a very detailed plan.
>
> > Again, these are details we can debate when the patches are ready for
> > discussion.
> >
> > But again, let's start with your patch set. What are the next steps in
> > taking it upstream?
> > And which are the next ops you would like to see implemented? I would
> > be happy to take
> > a stab at one or two.
> >
>
> I can post patches for passthrough getxattr/listxattr, those are pretty
> simple,

Spoke up too soon.
passthrough of getxattr is only allegedly simple - that's before realizing
the complexity of passthrough of get_acl (see ovl_get_acl()).

> but I am not sure if they have merit on their own without
> passthrough of getattr, which is more complicated.
>
> Also I am not sure that implementing passthrough of some inode ops
> has merit without being able to setup passthrough at lookup time.
>
> I will see if I can find time to post a POC of basic passthrough of inode
> ops and setup of backing id at lookup time.
>

I did not get far enough to post a POC, but I did get far enough
to test inode ops passthrough.

This branch [1] has patches that implement passthrough for readdir
and getattr/statx.

I mostly wanted to demonstrate that you do not need to invent any new
infra to support passthrough of inode/dir ops - all the infra is already
there just waiting for a richer API to setup inode ops passthrough
at lookup time and implement the different passthrough methods.
I hope that this is clear?

I ran into an ABI compatibility issue with extending the fuse_entry_out
lookup result, which is also being used in struct fuse_direntplus,
so for the demo, I worked around this problem by setting up the
iops passthrough on first GETATTR.
The ABI compatibility (with existing binaries) is solvable but requires
more work.

In this demo passthrough_fs [2], when run with arguments
--readdirpassthrough --nocache
the first GETATTR on every dir/file sets up a permanent
backing inode for the fuse inode and all the following GETATTR
calls are passed through on this inode (until drop_caches).

Readdir is also passed through, but because passthrough
of LOOKUP is not implemented, it is not a clear win to use
readdir/getattr passthrough in comparison to readdirplus.
Hence, there is no justification to post these patches on their own.
But you can use them as a basis for your work.

If you have any questions, don't hesitate to ask me.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/fuse_passthrough_iops/
[2] https://github.com/amir73il/libfuse/commits/fuse_passthrough_iops/

Powered by blists - more mailing lists