Re: [PATCH RFC 00/11] mm/filemap: split out folio wait and VFS code

[Christoph Hellwig]

On Fri, May 29, 2026 at 06:54:07PM -0400, Tal Zussman wrote:
> On 5/28/26 8:49 AM, Christoph Hellwig wrote:
> > On Thu, May 28, 2026 at 11:22:37AM +0200, Jan Kara wrote:
> >> Overall this makes sense to me. In particular I agree it makes sense to
> >> move the file read/write helpers into fs.
> > 
> > I disagree very strongly.  Mixing default implementations with the
> > higher level APIs is a really bad idea and leads to people taking
> > stupid shortcuts and other layering violations.
> 
> fs/read_write.c already contains some of these "generic" function
> implementations, including generic_write_checks(), which is called by
> generic_file_write_iter() in mm/filemap.c. Right now the two files are
> unnecessarily interdependent. I do think fs/read_write.c is the natural home
> for these functions.

generic_write_checks is a very different beast.  It is a generic helper
that every implementation must call. The implementations have to call
it with the right locks held, and this it can't be done before calling
into the method.

> > Splitting up filemap.c makes sense, but I'd rather keep the generic copy
> > into and out of the pagecache code with the MM infrastructure for it,
> > as it is not VFS code, and making that clear to anyone touching the code
> > is important.
> 
> About half the code moved is implementing direct I/O or multiplexing between
> page cache I/O and direct I/O.

This will hopefully change quite a bit once we move everyone off the
legacy direct I/O code and the helpers for it.  Another reason not to
move the code around for now as it should change a bit.

> It definitely shouldn't be in the page cache,
> and I do think it is VFS code.

For the higher level stuff I'd agree.  But I'm not sure how much
is left after the above.  If we have good helpers left something like
libfs.c or a new library might be a better place.

> The one exception I see is
> generic_perform_write(), which is analogous to filemap_read() and should stay
> in filemap.c (and probably be renamed to something like filemap_write()).

Agreed on that.