Re: [PATCH 5.15] fuse: Fix race condition in writethrough path A race

Bernd Schubert
2025-10-13T20:16:07Z

On 10/13/25 15:39, Miklos Szeredi wrote:
> On Fri, 10 Oct 2025 at 10:46, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
>> My idea is to introduce FUSE_I_MTIME_UNSTABLE (which would work
>> similarly to FUSE_I_SIZE_UNSTABLE) and when fetching old_mtime, verify
>> that it hasn't been invalidated. If old_mtime is invalid or if
>> FUSE_I_MTIME_UNSTABLE signals that a write is in progress, the page
>> cache is not invalidated.
>
> [Adding Brian Foster, the author of FUSE_AUTO_INVAL_DATA patches.
> Link to complete thread:
> https://lore.kernel.org/all/20251009110623.3115511-1-giveme.gulu@gmail.com/#r]
>
> In summary: auto_inval_data invalidates data cache even if the
> modification was done in a cache consistent manner (i.e. write
> through). This is not generally a consistency problem, because the
> backing file and the cache should be in sync. The exception is when
> the writeback to the backing file hasn't yet finished and a getattr()
> call triggers invalidation (mtime change could be from a previous
> write), and the not yet written data is invalidated and replaced with
> stale data.
>
> The proposed fix was to exclude concurrent reads and writes to the same region.
>
> But the real issue here is that mtime changes triggered by this client
> should not cause data to be invalidated. It's not only racy, but it's
> fundamentally wrong. Unfortunately this is hard to do this correctly.
> Best I can come up with is that any request that expects mtime to be
> modified returns the mtime after the request has completed.
>
> This would be much easier to implement in the fuse server: perform the
> "file changed remotely" check when serving a FUSE_GETATTR request and
> return a flag indicating whether the data needs to be invalidated or
> not.

For an intelligent server maybe, but let's say one uses
<libfuse>/example/passthrough*, in combination with some external writes
to the underlying file system outside of fuse. How would passthrough*
know about external changes?

The part I don't understand yet is why invalidate_inode_pages2() causes
an issue - it has folio_wait_writeback()?

Thanks,
Bernd

>
> Thoughts?
>
> Thanks,
> Miklos