Message-ID: <CAOQ4uxhYcNvQc-Y+ZZSGyX1Un8WCJuE-aeiRrgLm91HwJ48gWA@mail.gmail.com>
Date: Wed, 3 Jul 2024 17:41:53 +0300
From: Amir Goldstein <amir73il@...il.com>
To: Bernd Schubert <bernd.schubert@...tmail.fm>
Cc: Daniel Rosenberg <drosen@...gle.com>, Miklos Szeredi <miklos@...redi.hu>, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH 0/1] Fuse Passthrough cache issues

On Wed, Jul 3, 2024 at 4:27 PM Bernd Schubert
<bernd.schubert@...tmail.fm> 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@mail.gmail.com/

>
> 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?

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).

> I think idea for conflicting file cached and passthrough modes is that
> the backing inode can already provide a cache - why another one for fuse?
>
>
> >
> > The other issue I ran into is the restriction on opening a file in passthrough
> > and non passthrough modes. In Android, one of our main usecases for passthrough
> > is location metadata redaction. Apps without the location permission get back
> > nulled out data when they read image metadata location. If an app has that
> > permission, it can open the file in passthrough mode, but otherwise the daemon
> > opens the file in normal fuse mode where it can do the redaction.
> >
> > Currently in passthrough, this behavior is explicitly blocked. What's needed to
> > allow this? The page caches can contain different data, but in this case that's
> > a feature, not a bug. They could theoretically be backed by entirely different
> > things, although that would be fairly confusing. I would think the main thing
> > we'd need would be to invalidate areas of the cache when writing in passthrough
> > mode to give the daemon the opportunity to react to what's there now, and also
> > something in the other direction. Might make more sense as something the daemon
> > can opt into.
> >
> > Any thoughts on these issues? And does the proposed fix make sense to you?

FYI, Miklos suggested that attributes will be passthrough to the "backing inode"
when inode is in passthrough mode, which requires lookup-to-forget semantics
just like in the BPF patches.

I have started a POC of this [2], but got sidetracked to other things.
I am not sure when I will be able to get back to them.
If this is interesting to you either for solving the specific attribute mismatch
issue or as a stepping stone towards FUSE BPF, do feel free to pick up
my patches.

[2] https://github.com/amir73il/linux/commits/fuse-backing-inode-wip/

Let me know if you have any questions about them.

Thanks,
Amir.

Powered by blists - more mailing lists