Initialize parity blocks before RAID-Z reconstruction benchmarking #12473
Conversation
This file contains 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
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Signed-off-by: Mark Johnston <markj@FreeBSD.org> Sponsored-by: The FreeBSD Foundation
behlendorf
approved these changes
Aug 13, 2021
There was a problem hiding this comment.
Nice find, this looks good. I think it's probably fine to stick with the memset() of the 0xAA pattern here to be consistent with how the bench_zio->io_abd is allocated. We could simplify things here a little bit and use abd_zero() in both cases, but for this either way is fine with me.
behlendorf
added
Status: Code Review Needed
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Aug 23, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Aug 24, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Aug 24, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Aug 24, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
behlendorf
pushed a commit
that referenced
this pull request
Aug 31, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes #12473
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 15, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
rincebrain
pushed a commit
to rincebrain/zfs
that referenced
this pull request
Sep 22, 2021
benchmark_raidz() allocates a row to benchmark parity calculation and reconstruction. In the latter case, the parity blocks are left uninitialized, leading to reports from KMSAN. Initialize parity blocks to 0xAA as we do for the data earlier in the function. This does not affect the selected RAID-Z implementation on any of several systems tested. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Mark Johnston <markj@FreeBSD.org> Closes openzfs#12473
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.
During initialization, ZFS benchmarks the available RAID-Z implementations to choose the fastest one. When it does so, it allocates a ZIO, initializes the backing buffer, and allocates a row used in the benchmark. In particular, it allocates buffers for the parity blocks but does not initialize them, so the benchmark runs on uninitialized data, leading to spurious reports from KMSAN.
Motivation and Context
Without this change, ZFS in a KMSAN-enabled kernel triggers false positives during initialization.
Description
This change initializes the parity buffers to 0xAA before testing reconstruction.
How Has This Been Tested?
Manual testing, verified that the KMSAN report goes away and that the selected RAID-Z math implementation isn't affected by this change.
Types of changes
Checklist:
Signed-off-by.