Re: [PATCH RFCv2 2/4] iomap: optional zero range dirty folio processing

[Christoph Hellwig <hch@infradead.org>]

On Fri, Jan 10, 2025 at 12:53:19PM -0500, Brian Foster wrote:
> processing.
> 
> For example, if we have a largish dirty folio backed by an unwritten
> extent with maybe a single block that is actually dirty, would we be
> alright to just zero the requested portion of the folio as long as some
> part of the folio is dirty? Given the historical ad hoc nature of XFS
> speculative prealloc zeroing, personally I don't see that as much of an
> issue in practice as long as subsequent reads return zeroes, but I could
> be missing something.

That's a very good question I haven't though about much yet.  And
everytime I try to think of the speculative preallocations and they're
implications my head begins to implode..

> > >  static inline void iomap_iter_reset_iomap(struct iomap_iter *iter)
> > >  {
> > > +	if (iter->fbatch) {
> > > +		folio_batch_release(iter->fbatch);
> > > +		kfree(iter->fbatch);
> > > +		iter->fbatch = NULL;
> > > +	}
> > 
> > Does it make sense to free the fbatch allocation on every iteration,
> > or should we keep the memory allocation around and only free it after
> > the last iteration?
> > 
> 
> In the current implementation the existence of the fbatch is what
> controls the folio lookup path, so we'd only want it for unwritten
> mappings. That said, this could be done differently with a flag or
> something that indicates whether to use the batch. Given that we release
> the folios anyways and zero range isn't the most frequent thing, I
> figured this keeps things simple for now. I don't really have a strong
> preference for either approach, however.

I was just worried about the overhead of allocating and freeing
it all the time.  OTOH we probably rarely have more than a single
extent to process with the batch right now.