On Thu, Jun 19, 2025 at 9:50 PM Paul Lawrence <paullawrence@xxxxxxxxxx> 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, 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. Thanks, Amir.
