Re: [PATCH 1/2] iov_iter: iterate_folioq: fix handling of offset >= folio size

[asmadeus@codewreck.org]

David Howells wrote on Mon, Aug 11, 2025 at 03:37:43PM +0100:
> Dominique Martinet via B4 Relay wrote:
> > It's apparently possible to get an iov forwarded all the way up to the
> 
> By "forwarded" I presume you mean "advanced"?

Thanks, swapped words in v2

> > This should have been because we're only in the 2nd slot and there's
> > another one after this, but iterate_folioq should not try to map a
> > folio that skips the whole size, and more importantly part here does
> > not end up zero (because 'PAGE_SIZE - skip % PAGE_SIZE' ends up
> > PAGE_SIZE and not zero..), so skip forward to the "advance to next
> > folio" code.
> 
> Note that things get complicated because folioqs form a segmented list that
> can be under construction as it advances.  So if there's no next folioq
> segment at the time you advance to the end of the current one, it will end up
> parked at the end of the last folio or with slot==nr_slots because there's
> nowhere for it to advance to.

Hmm, I've already sent a v2 with other things fixed but now you made me
look at the "we're at the end of the iov_iter" case I think this won't
work well either?
folioq_folio() always returns something, and the advance code only
advances if folioq->next is set and doesn't bail out if it's unset.

There should be a `if (slot == folioq_nr_slots(folioq)) break` check
somewhere as well? Or is the iov_iter guaranteed to always 1/ have some
data and 2/ either be big enough or have remaining data in a step?

I can believe the former but wouldn't trust the later...

> Note that extract_folioq_to_sg() already does this as does
> iov_iter_extract_folioq_pages().

Yes we're not quite consistent here, some functions like the plain
iov_iter_advance will get you on an invalid slot to check for
folioq->next on next invocations while others point at the end of the
last folio in the queue (like iov_iter_extract_folioq_pages(), and
iov_folioq_get_pages() before patch 2);
I think either pattern is valid; I've changed iov_folioq_get_pages()
because it was a bit weird to have an iov_iter with offset > count and
iov_iter_advance wouldn't do this, but I agree either should work, we
just probably want to be more consistent.

Thanks,
-- 
Dominique Martinet | Asmadeus