Fix l2arc_apply_transforms ztest crash #15248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Paul Dagnelie <[email protected]>
rincebrain
reviewed
Sep 8, 2023
|
There's no specific tests around the EA code, but all the l2arc compressed_arc disabled tests were what was exploding then, which was likely just this problem, but that fix happened to work because it wasn't simulating a mixed ashift environment. So I think you could probably make a trivial modification of those tests to test this breaking with the previous patch reverted in both mixed ashift and not cases, if you'd like, watch that explode, then watch one but not the other explode with current git, and both not explode with this. |
Signed-off-by: Paul Dagnelie <[email protected]>
mmaybee
approved these changes
Sep 14, 2023
behlendorf
approved these changes
Sep 19, 2023
behlendorf
added
Status: Accepted
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Sep 20, 2023
In openzfs#13375 we modified the allocation size of the buffer that we use to apply l2arc transforms to be the size of the arc hdr we're using, rather than the allocation size that will be in place on the disk, because sometimes the hdr size is larger. Unfortunately, sometimes the allocation size is larger, which means that we overflow the buffer in that case. This change modifies the allocation to be the max of the two values Reviewed-by: Mark Maybee <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Paul Dagnelie <[email protected]> Closes openzfs#15177 Closes openzfs#15248
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Sep 21, 2023
In openzfs#13375 we modified the allocation size of the buffer that we use to apply l2arc transforms to be the size of the arc hdr we're using, rather than the allocation size that will be in place on the disk, because sometimes the hdr size is larger. Unfortunately, sometimes the allocation size is larger, which means that we overflow the buffer in that case. This change modifies the allocation to be the max of the two values Reviewed-by: Mark Maybee <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Paul Dagnelie <[email protected]> Closes openzfs#15177 Closes openzfs#15248
behlendorf
pushed a commit
that referenced
this pull request
Sep 22, 2023
In #13375 we modified the allocation size of the buffer that we use to apply l2arc transforms to be the size of the arc hdr we're using, rather than the allocation size that will be in place on the disk, because sometimes the hdr size is larger. Unfortunately, sometimes the allocation size is larger, which means that we overflow the buffer in that case. This change modifies the allocation to be the max of the two values Reviewed-by: Mark Maybee <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Paul Dagnelie <[email protected]> Closes #15177 Closes #15248
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Dec 12, 2023
In openzfs#13375 we modified the allocation size of the buffer that we use to apply l2arc transforms to be the size of the arc hdr we're using, rather than the allocation size that will be in place on the disk, because sometimes the hdr size is larger. Unfortunately, sometimes the allocation size is larger, which means that we overflow the buffer in that case. This change modifies the allocation to be the max of the two values Reviewed-by: Mark Maybee <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Paul Dagnelie <[email protected]> Closes openzfs#15177 Closes openzfs#15248
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
We have a crash ocurring pretty regularly in zloop that indicates corruption in the buffer allocated by
l2arc_apply_transforms. This issue is documented in #15177.Description
In #13375 we modified the allocation size of the buffer that we use to apply l2arc transforms to be the size of the arc hdr we're using, rather than the allocation size that will be in place on the disk, because sometimes the hdr size is larger. Unfortunately, sometimes the allocation size is larger, which means that we overflow the buffer in that case. This change modifies the allocation to be the max of the two values.
How Has This Been Tested?
Two hours of zloop runs with asan, and the issue did not recur. Previously it reliably occurred within 30 minutes. I'm not sure of the easiest way to test to make sure that the original issue is still fixed; @rincebrain is there a test in the test suite that covers it? If not I can try to do some manual testing of the early abort logic.
Types of changes
Checklist:
Signed-off-by.