Re: [PATCH 17/21] xfs: add file_{get,put}_folio

Darrick J. Wong
2024-01-26T16:55:49Z

On Fri, Jan 26, 2024 at 04:40:57PM +0000, Matthew Wilcox wrote:
> On Fri, Jan 26, 2024 at 02:28:59PM +0100, Christoph Hellwig wrote:
> > + /*
> > + * Mark the folio dirty so that it won't be reclaimed once we drop the
> > + * (potentially last) reference in xfile_put_folio.
> > + */
> > + if (flags & XFILE_ALLOC)
> > + folio_set_dirty(folio);
>
> What I can't tell from skimming the code is whether we ever get the folio
> and don't modify it. If we do, it might make sense to not set dirty here,
> but instead pass a bool to xfile_put_folio(). Or have the caller dirty
> the folio if they actually modify it. But perhaps that never happens
> in practice and this is simple and works every time.

Generally we won't be allocating an xfile folio without storing data to it.
It's possible that there could be dirty folios containing zeroes (e.g.
an xfarray that stored a bunch of array elements then nullified all of
them) but setting dirty early is simpler.

(and all the users of xfiles are only interested in ephemeral datasets
so most of the dirty pages and the entire file will get flushed out
quickly)

--D