Re: [PATCH v1 16/16] fuse: remove fuse_readpages_end() null mapping check

[Joanne Koong <joannelkoong@gmail.com>]

On Tue, Sep 2, 2025 at 2:22 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Sat, 30 Aug 2025 at 01:58, Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > Remove extra logic in fuse_readpages_end() that checks against null
> > folio mappings. This was added in commit ce534fb05292 ("fuse: allow
> > splice to move pages"):
> >
> > "Since the remove_from_page_cache() + add_to_page_cache_locked()
> > are non-atomic it is possible that the page cache is repopulated in
> > between the two and add_to_page_cache_locked() will fail.  This
> > could be fixed by creating a new atomic replace_page_cache_page()
> > function.
> >
> > fuse_readpages_end() needed to be reworked so it works even if
> > page->mapping is NULL for some or all pages which can happen if the
> > add_to_page_cache_locked() failed."
> >
> > Commit ef6a3c63112e ("mm: add replace_page_cache_page() function") added
> > atomic page cache replacement, which means the check against null
> > mappings can be removed.
>
> If I understand correctly this is independent of the patchset and can
> be applied without it.

Yes, this and patch 05/16 ("iomap: propagate iomap_read_folio() error
to caller"), patch 08/16 ("iomap: rename iomap_readpage_iter() to
iomap_readfolio_iter()"), and patch 09/16 ("iomap: rename
iomap_readpage_ctx struct to iomap_readfolio_ctx") in the series are
independent from fuse iomap read/readahead functionality.

My thinking was that it would be more cohesive to have everything in
one place so that there's less patches scattered about, but I'm
realizing now it probably was just more confusing than helpful.

Thanks,
Joanne

>
> Thanks,
> Miklos