On Wed, Jul 3, 2024 at 7:09 PM Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote: > > > > On 7/3/24 16:41, Amir Goldstein wrote: > > On Wed, Jul 3, 2024 at 4:27 PM Bernd Schubert > > <bernd.schubert@xxxxxxxxxxx> wrote: > >> > >> > >> > >> On 7/3/24 03:02, Daniel Rosenberg wrote: > >>> I've been attempting to recreate Android's usage of Fuse Passthrough with the > >>> version now merged in the kernel, and I've run into a couple issues. The first > >>> one was pretty straightforward, and I've included a patch, although I'm not > >>> convinced that it should be conditional, and it may need to do more to ensure > >>> that the cache is up to date. > >>> > >>> If your fuse daemon is running with writeback cache enabled, writes with > >>> passthrough files will cause problems. Fuse will invalidate attributes on > >>> write, but because it's in writeback cache mode, it will ignore the requested > >>> attributes when the daemon provides them. The kernel is the source of truth in > >>> this case, and should update the cached values during the passthrough write. > >> > >> Could you explain why you want to have the combination passthrough and > >> writeback cache? > >> > >> I think Amirs intention was to have passthrough and cache writes > >> conflicting, see fuse_file_passthrough_open() and > >> fuse_file_cached_io_open(). > > > > Yes, this was an explicit design requirement from Miklos [1]. > > I also have use cases to handle some read/writes from server > > and the compromise was that for the first version these cases should > > use FOPEN_DIRECT_IO, which does not conflict with FOPEN_PASSTHROUGH. > > > > I guess this is not good enough for Android applications opening photos > > that need the FUSE readahead cache for performance? > > > > In that case, a future BPF filter can decide whether to send the IO direct > > to server or to backing inode. > > > > Or a future backing inode mapping API could map part of the file to > > backing inode > > and the metadata portion will not be mapped to backing inode will fall back to > > direct IO to server. > > > > [1] https://lore.kernel.org/linux-fsdevel/CAJfpegtWdGVm9iHgVyXfY2mnR98XJ=6HtpaA+W83vvQea5PycQ@xxxxxxxxxxxxxx/ > > > >> > >> Also in <libfuse>/example/passthrough_hp.cc in sfs_init(): > >> > >> /* Passthrough and writeback cache are conflicting modes */ > >> > >> > >> > >> With that I wonder if either fc->writeback_cache should be ignored when > >> a file is opened in passthrough mode, or if fuse_file_io_open() should > >> ignore FOPEN_PASSTHROUGH when fc->writeback_cache is set. Either of both > >> would result in the opposite of what you are trying to achieve - which > >> is why I think it is important to understand what is your actual goal. > >> > > > > Is there no standard way for FUSE client to tell the server that the > > INIT response is invalid? > > Problem is that at FUSE_INIT time it is already mounted. process_init_reply() > can set an error state, but fuse_get_req*() will just result in ECONNREFUSED.* > > > > > Anyway, we already ignore FUSE_PASSTHROUGH in INIT response > > for several cases, so this could be another case. > > Then FOPEN_PASSTHROUGH will fail with EIO (not be ignored). > > So basically this? > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index 573550e7bbe1..36c6dcd47a53 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -1327,7 +1327,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args, > if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) && > (flags & FUSE_PASSTHROUGH) && > arg->max_stack_depth > 0 && > - arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH) { > + arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH && > + !(flags & FUSE_WRITEBACK_CACHE)) { > fc->passthrough = 1; > fc->max_stack_depth = arg->max_stack_depth; > fm->sb->s_stack_depth = arg->max_stack_depth; > > Yap. Maybe add something to the comment, because the comment is about max_stack_depth and someone may assume that it also refers to FUSE_WRITEBACK_CACHE somehow. Thanks, Amir.
