On Wed, Sep 13, 2023 at 11:29 AM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > On Tue, Sep 12, 2023 at 09:54:08PM +0300, Amir Goldstein wrote: > > Overlayfs stores its files data in backing files on other filesystems. > > > > Factor out some common helpers to perform io to backing files, that will > > later be reused by fuse passthrough code. > > > > Suggested-by: Miklos Szeredi <miklos@xxxxxxxxxx> > > Link: https://lore.kernel.org/r/CAJfpeguhmZbjP3JLqtUy0AdWaHOkAPWeP827BBWwRFEAUgnUcQ@xxxxxxxxxxxxxx > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > --- > > > > Miklos, > > > > This is the re-factoring that you suggested in the FUSE passthrough > > patches discussion linked above. > > > > This patch is based on the overlayfs prep patch set I just posted [1]. > > > > Although overlayfs currently is the only user of these backing file > > helpers, I am sending this patch to a wider audience in case other > > filesystem developers want to comment on the abstraction. > > > > We could perhaps later considering moving backing_file_open() helper > > and related code to backing_file.c. > > > > In any case, if there are no objections, I plan to queue this work > > for 6.7 via the overlayfs tree. > > > > Thanks, > > Amir. > > > > [1] https://lore.kernel.org/linux-unionfs/20230912173653.3317828-1-amir73il@xxxxxxxxx/ > > > > > > MAINTAINERS | 2 + > > fs/Kconfig | 4 + > > fs/Makefile | 1 + > > fs/backing_file.c | 160 +++++++++++++++++++++++++++++++++++ > > I'm sorry but I'm missing mountains of context. > How is that related to the backing file stuff exactly? > The backing file stuff has this unpleasant > > file->f_inode == real_inode != file->f_path->dentry->d_inode > > that we all agree is something we really don't like. Is FUSE trying to > do the same thing and build an read_iter/write_iter abstraction around > it? I really really hope that's not the case. That is not the case. The commonality between FUSE passthrough and overlayfs is that a "virtual" file (i.e. ovl/fuse), which has no backing blockdev of its own "forwards" the io requests to a backing file on another filesystem. The name "backing file" is therefore a pretty accurate description for both cases. HOWEVER, FUSE does not need to use the backing_file struct to hold an alternative path, so FUSE backing files do not have FMODE_BACKING, same as cachefiles uses backing files, but does not use the FMODE_BACKING/file_backing struct. Yes, it's a bit of a naming mess. I don't have any good ideas on how to do better naming. Ideally, we will get rid of struct backing_file, so we won't need to care about the confusing names... > > And why are we rushing this to a VFS API? This should be part of the > FUSE series that make it necessary to hoist into the VFS not in > overlayfs work that prematurely moves this into the VFS. Fair enough, I will not rush this patch and will post it along with the FUSE passthrough patches. I posted it because I wanted to get early feedback - mission accomplished :) In retrospect, I should have labeled it [RFC]. Thanks, Amir.
