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; Thanks, Bernd
