Skip to content

Fix two issues with dynamic gang headers#17587

Closed
pcd1193182 wants to merge 2 commits intoopenzfs:masterfrom
pcd1193182:gang_fix
Closed

Fix two issues with dynamic gang headers#17587
pcd1193182 wants to merge 2 commits intoopenzfs:masterfrom
pcd1193182:gang_fix

Conversation

@pcd1193182
Copy link
Copy Markdown
Contributor

@pcd1193182 pcd1193182 commented Aug 1, 2025

Sponsored by: [Klara, Inc.; Wasabi Technology, Inc.]

Motivation and Context

When running some tests on a heavily fragmented system using raidz, I noticed a large number of checksum errors. I investigated, and determined that these were occurring when gang blocks were created. I modified the gang block tests to use raidz instead of plain vdevs, and the issue occurred in the tests immediately.

Description

First, there are test changes to better test more configurations. Most of the test changes are just indentation, but the calls to check_not_gang_dva have been replaced with calls to the new functions check_gang_bp and check_not_gang_bp respectively, which is what should actually have been tested there.

Next, there is a fix in zio_checksum.c for the case where there are more intermediate ZIOs between the one being checksummed and the gang header. This occurred in mirror devices in my testing, but would also occur if you had raidz devices being replaced, or possibly indirect vdevs.

Finally, there is a fix in zio.c for the raidz code. The problem we were running into in that case is that we were using the asize of the whole allocation used by the gang header as the psize of the IO, which resulted in an even larger read/write being done. We need to convert back down from the gang_header_asize to the actual psize that write provides.

How Has This Been Tested?

Ran the updated gang block tests, verified that on the original system where I saw the issue this prevented checksum errors from occurring.

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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • 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:

Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 1, 2025
Copy link
Copy Markdown
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's subtle, but the fix makes good sense. As does extending the test case to cover raidz and draid.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 1, 2025
Comment thread module/zfs/zio.c Outdated
@github-actions github-actions Bot removed the Status: Accepted Ready to integrate (reviewed, tested) label Aug 4, 2025
Comment thread module/zfs/zio.c Outdated
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
@amotin amotin added the Status: Accepted Ready to integrate (reviewed, tested) label Aug 5, 2025
@behlendorf behlendorf closed this in 8ecf044 Aug 6, 2025
behlendorf pushed a commit that referenced this pull request Aug 6, 2025
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes #17587
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
pcd1193182 added a commit to KlaraSystems/zfs that referenced this pull request Nov 13, 2025
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
pcd1193182 added a commit to KlaraSystems/zfs that referenced this pull request Nov 13, 2025
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#17587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Accepted Ready to integrate (reviewed, tested)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants