abd_iter_page: rework to handle multipage scatterlists

[robn]

Motivation and Context

@bwatkinson reported assertion trips and memory corruption after rebasing #10018 onto recent master. He traced the problem to scatterlists received from userspace carrying multiple contiguous non-compound pages, and asked me about it.

Description

Previously, abd_iter_page() would assume that every scatterlist would contain a single page (compound or no), because that's all we ever create in abd_alloc_chunks(). However, scatterlists can contain multiple pages of arbitrary provenance, and if we get one of those, we'd get all the math wrong.

This reworks things to handle multiple pages in a scatterlist, by properly finding the right page within it for the given offset, and understanding better where the end of the page is and not crossing it.

How Has This Been Tested?

Full ZTS run completed successfully against kernel 6.1.76.

Compile checked, sanity tested and "known troublesome" tests (ie zpool_reopen_003_pos) passed on 3.10-EL7, 4.14.336, 5.10.214, 6.1.83 and 6.8.2.

Using @bwatkinson's rebase of #10018 to master, the -T direct test suite asserts out early without this change (specifically the dio_aligned_block test), and all tests pass with it. See robn/directio-test.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[robn]

@behlendorf @tonyhutter fwiw, assuming this passes muster, I'm softly against taking it to 2.2.4 at the last minute. While the scatterlist page iterator is not quite right, I'm pretty sure that it works properly for the only kinds of scatterlists it can currently receive, which should all be compound, and this code is off by default on 2.2 anyway. On the other hand, we know how many weird and wild Linux variants are out there, and we wanted this on 2.2 so we could ask people already hitting peculiar cases to try it, so one could argue they're already in a weird world. So I could go either way, but my default position is to err towards caution. I won't argue if you wanted to go the other way.

[tonyhutter]

 * Disable compound pages before kernel 4.5. See comment below for why.

Just crudely looking this over - is this something that only affects < 4.5 kernels? Only asking since CentOS 7 is going to go EOL in June 2024...

[robn]

Just crudely looking this over - is this something that only affects < 4.5 kernels? Only asking since CentOS 7 is going to go EOL in June 2024...

@tonyhutter No, all kernels. The comments and defines are just moving some of the logic from the existing 4.5 check up, because I need the newer ABD_PAGE_SIZE macro. The explainer is down in the comment; I tried moving it up but it didn't entirely make sense out of context.

You're right though, its not easy to follow though. I'll rework the commentary (code stays the same).

[robn]

I'll rework the commentary (code stays the same).

Pushed update, lmk!

[bwatkinson]

I can confirm this PR does fix the issue I was encountering in the Direct I/O PR (#10018).