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,