Revert commit to fix LUKS write errors

tonyhutter commented 1 month ago

Motivation and Context

Workaround ZFS on LUKS write errors on master branch.

Description

Revert bd7a02c251d8c119937e847d5161b512913667e6 as a workaround for #16631 and add a LUKS test case. This revert is more of a stop-gap for 2.3.x while we come up with a proper fix for #16631.

How Has This Been Tested?

Types of changes

[x] 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.

amotin commented 1 month ago

Lets not hurry with this. If needed, we can use smaller workaround patch from https://github.com/openzfs/zfs/issues/16631#issuecomment-2430232917 .

robn commented 1 month ago

I'm not opposed to reverting (we haven't had this for most of 2.2 so it's a known quantity at least). I would prefer @amotin's far smaller patch if we think it covers enough (sounds right to me right now).

I will note that this will likely silence the uptick of recent issues, but it's not going to work around all related issues with LUKS, which have been seen in places that didn't have the the allocator change this patch reverts (late 2.2, and 2.1), because 4K blocks split across memory pages are still possible (gang ABDs, LINEAR_PAGE scatterlists). That's ok, reverting to a known and smaller broken is sensible, I just wanted anyone with an interest in those older problem to not get too excited by this change (based on the PR name).

tonyhutter commented 1 month ago

I'm putting this PR on hold in favor of the fix here: https://github.com/openzfs/zfs/issues/16631#issuecomment-2430232917

robn commented 1 month ago

Thanks @tonyhutter :crown:

tonyhutter commented 1 month ago

I'm closing this in favor of the workaround PR (https://github.com/openzfs/zfs/pull/16678) and the standalone test case PR (https://github.com/openzfs/zfs/pull/16681).

openzfs / zfs