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

From: Miklos Szeredi <miklos@szeredi.hu>
To: Amir Goldstein <amir73il@gmail.com>
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 12:32:32 +0100	[thread overview]
Message-ID: <CAJfpegtr1yOYKOW0GLkow_iALMc_A0+CUaErZasQunAfJ7NFzw@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxiBu8bZ4URhwKuMeHB_Oykz2LHY8mXA1eB3FBoeM_Vs6w@mail.gmail.com>

On Tue, 31 Oct 2023 at 18:44, Amir Goldstein <amir73il@gmail.com> wrote:

> In that case, we would be able to "attach" the fuse_backing object
> to fuse_inode on CREATE response. If we end up with a backing map
> conflict at this point, we can return -EBUSY error to the user and forget
> the inode, but the server would have no easy feedback on its mistake.
> Also, -EBUSY to user would be confusing if user did not request O_EXCL.

I think -EIO is more appropriate.  Server is broken, WARN_ON_ONCE
could also be used to indicate that.

> Do you consider the described "atomic_open conflict" case an API problem?
>
> It is true that with v14 patches that do not enforce backing inode conflicts,
> the attribute coherency model that I proposed may result in attribute
> cache thrashing if the backing inode of a fuse inode is ambiguous.
>
> Do you have an idea how to handle this case elegantly?

Let me add some perspective.

Currently we have FOPEN_DIRECT_IO that disables caching.  My feeling
when dealing with this interface is that it was a mistake to make this
a property of the open file.  It should insted be a property of the
inode and all open file instances should respect this property
equally.  It makes no sense to have one file do cached reads while the
other is doing direct writes, etc.  Also it should be possible to turn
this on or off for all open file instances at any time.

Passthrough is similar, I think.  At any point in time all I/O should either be

 - cached
 - direct
 - passthrough to a specific backing file

Allowing these to be mixed leads to confusing semantics, especially
cached and non-cached

OTOH allowing passthrough to be switched on at any point in time
presents challenges.   If passthrough is turned on for an inode that
didn't have it previously means that the backing file needs to be set
up before the actual I/O.    So it's definitely more complex than just
setting up the backing at open.  But I feel that longer term we'll end
up with a better interface.

For the first version we can just bypass this whole mess, and say that
passthrough can only be turned on for the first open.  I.e. if there
are already open instances and passthrough has not yet been set up,
then FOPEN_PASSTHROUGH will be ignored.  Similarly if it has already
been set up, then the lack of FOPEN_PASSTHROUGH is also ignored.

Hmm?

Thanks,
Miklos