On Wed, Sep 13, 2023 at 2:24 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > 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... > Alas, my explanation about FUSE passthrough backing files turned out to be inaccurate. FUSE IO passthrough to backing file is very similar to overlayfs IO "passthrough" to lower/upper backing file. Which creates the same problem w.r.t mmap'ing the FUSE file to the underlying backing file inode. That problem in overlayfs caused the inception of the fake path file now embedded in the backing_file object. So yes, FUSE is trying to do the same thing. However, the helpers in this patch are not dealing with the fake path aspect of those backing files specifically. They are common code for a "stacked fs" (i.e. s_stack_depth > 0) which delegates IO to files on the underlying fs. I will give it some thought on how we can at least narrow the amount of filesystem code that could be exposed to those fake path files. The read/write/splice iterators do not really need to operate of fake path files, so this is something that I can try to avoid. Thanks, Amir.
