Re: [PATCH v14 00/12] FUSE passthrough for file io

From: Amir Goldstein <amir73il@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Bernd Schubert <bernd.schubert@fastmail.fm>,
	Daniel Rosenberg <drosen@google.com>,
	 Paul Lawrence <paullawrence@google.com>,
	Alessio Balsini <balsini@android.com>,
	 Christian Brauner <brauner@kernel.org>,
	fuse-devel@lists.sourceforge.net,  linux-fsdevel@vger.kernel.org,
	Dave Chinner <david@fromorbit.com>
Subject: Re: [PATCH v14 00/12] FUSE passthrough for file io
Date: Wed, 1 Nov 2023 20:32:29 +0200	[thread overview]
Message-ID: <CAOQ4uxipyZOSMcko+V+ZxGZwAgKVwWTUeoH79zqtMqbcKSnOoA@mail.gmail.com> (raw)
In-Reply-To: <CAJfpegt3mEii075roOTk6RKeNKGc89pGMkWrvVM0uLyrpg7Ebg@mail.gmail.com>

On Wed, Nov 1, 2023 at 5:25 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, 1 Nov 2023 at 16:06, Amir Goldstein <amir73il@gmail.com> wrote:
>
> > That would make sense if readdirplus request is sent to server
> > in parallel to executing readdir passthrough and readdirplus
> > response does the prepopulation asynchronously.
>
> Yes, that's fine, and it might even make sense to do that regardless
> of passthrough.  Readdirplus is similar to read-ahead in this regard.
>
> > Are you also referring to mixing cached and uncached readdir?
> > because that seems fine to me.
>
> But is it really fine?  The server must be very careful to prevent
> inconsistency between the cached and the uncached readdir, which could
> lead to really surprising results.  I just see no point in caching if
> the same result is available from a backing directory.
>

I see. so if dir has backing inode, we can always
disable FOPEN_CACHE_DIR (as my patch already does)
but instead of alway doing passthrough readdir, we employ the
heuristic whether or not to do readdirplus.
If not doing readdirplus, we can do fuse_passthrough_readdir().
In the future, we could do async readdirplus and always call
fuse_passthrough_readdir() if we have backing inode.

For now, I will just remove the readdir passthough patch.

> > I will try to add these rules to FOPEN_PASSTHROUGH:
> > - ignore request on backing inode conflict
> > - ignore request if inode is already open in caching mode
> > - make FOPEN_PASSTHROUGH implicit if inode is already open
> >   in passthrough *unless* file is open with FOPEN_DIRECT_IO
>
> Sounds good.
>
> There's still this case:
>
>  - file is opened in non-passthrough caching mode, and is being read/written
>  - at the same time the file is opened in passthrough mode
>
> The proper thing would be to switch the first file to passthrough mode
> while it's open.  It adds some complexity but I think it's worth it.

Remember that we would actually need to do backing_file_open()
for all existing open files of the inode.

Also, after the server calls FUSE_DEV_IOC_BACKING_CLOSE,
will the fuse_backing object still be referenced from the inode
(as it was in v13)? and then properly closed only on the last file
close on that inode?

Otherwise, new file opens without explicit FOPEN_PASSTHROUGH
will not have a fuse_backing object to open the backing_file from.

I am not convinced that this complexity is a must for the first version.
If the server always sets FOPEN_PASSTHROUGH (as I expect many
servers will) this is unneeded complexity.

It seems a *lot* easier to do what you suggested and ignore
FOPEN_PASSTHROUGH if the server is not being consistent
with the FOPEN_ flags it uses.

Thanks,
Amir.