Re: [PATCH 03/11] iomap: refactor the writeback interface

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 17, 2025 at 11:33:31AM -0700, Joanne Koong wrote:
> > +               struct folio *folio, u64 pos, unsigned int len, u64 end_pos);
> 
> end_pos only gets used in iomap_add_to_ioend() but it looks like
> end_pos can be deduced there by doing something like "end_pos =
> min(folio_pos(folio) + folio_size(folio), i_size_read(wpc->inode))".
> Would it be cleaner for ->writeback_range() to just pass in pos and
> len instead of also passing in end_pos? I find the end_pos arg kind of
> confusing anyways, like I think most people would think end_pos is the
> end of the dirty range (eg pos + len), not the end position of the
> folio.

i_size could change under us.  See commit b44679c63e4d ("iomap: pass byte
granular end position to iomap_add_to_ioend") which addes this end_pos
passing for details.

> > -       return 0;
> > +map_blocks:
> 
> nit: should this be called map_blocks or changed to something like
> "add_to_ioend"? afaict, the mapping has already been done by this
> point?

Sure.  Or maybe I just need to refactor the code to keep a separate
map_blocks helper and wrap it in a writeback_range one to make things a
bit easier to follow.