Just a bit of nitpicking, otherwise this looks sane, although I'd
want to return to proper review of the squashed prep patches first.
> if (!mapping_large_folio_support(iter->inode->i_mapping))
> len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
>
> + if (iter->fbatch) {
> + struct folio *folio = folio_batch_next(iter->fbatch);
> + if (folio) {
Missing empty line after the variable declaration.
> + if (!folio) {
> + WARN_ON_ONCE(!iter->fbatch);
> + len = 0;
> + goto out;
Given that the put label really just updates the return by reference
folio and len arguments we might as well do that here and avoid the
goto.
> + } else if (folio_pos(folio) > iter->pos) {
Also no need for an else after a goto (or return).
> @@ -1374,6 +1393,11 @@ static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
> if (iter->iomap.flags & IOMAP_F_STALE)
> break;
>
> + if (!folio) {
> + iomap_iter_advance(iter, iomap_length(iter));
> + break;
> + }
Maybe throw in a comment here how the NULL folio can happen?
> + while (filemap_get_folios(mapping, &start, end, &fbatch) &&
> + folio_batch_space(iter->fbatch)) {
> + struct folio *folio;
> + while ((folio = folio_batch_next(&fbatch))) {
> + if (folio_trylock(folio)) {
> + bool clean = !folio_test_dirty(folio) &&
> + !folio_test_writeback(folio);
> + folio_unlock(folio);
> + if (clean)
> + continue;
> + }
> +
> + folio_get(folio);
> + if (!folio_batch_add(iter->fbatch, folio)) {
> + end_pos = folio_pos(folio) + folio_size(folio);
> + break;
> + }
> + }
> + folio_batch_release(&fbatch);
I think I mentioned this last time, but I'd much prefer to do away
with the locla fbatch used for processing and rewrite this using a
find_get_entry() loop. That probably means this helper needs to move
to filemap.c, which should be easy if we pass in the mapping and outer
fbatch.
> + if (WARN_ON_ONCE(iter.fbatch && srcmap->type != IOMAP_UNWRITTEN))
Overly long line.
> 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?
