Skip to content

Fix bug caused by rounding in vdev_raidz_asize_to_psize #17490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2025

Conversation

pcd1193182
Copy link
Contributor

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.

Motivation and Context

Following @robn 's fix in #17488, I was running some tests with raidz and gang blocks, and ran into a kernel panic. My investigation turned up the following:

When an allocation is happening on a raidz vdev, the number of sectors allocated is rounded up to a multiple of nparity + 1. If this results in the allocation spilling into an extra row, then the corresponding call to vdev_raidz_asize_to_psize will incorrectly assume that parity sectors were allocated for that spilled row, even though no data is stored there.

Description

If we determine that happened, we need to subtract out those extra sectors before performing the rest of the capacity calculation.

How Has This Been Tested?

Manual testing on the system where I found the issue, mathematical analysis with a spreadsheet to verify before/after behavior of the updated formula.

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:

@allanjude allanjude requested review from amotin and robn June 26, 2025 23:46
Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

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

I think looks right (with my <24hr-old expert hat on).

Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

The math seems to make sense, but I am not sure I can make sense out of "don't subtract out parity for that row" comment. We are not just not subtracting parity, we ignore it at all.

@amotin amotin added the Status: Accepted Ready to integrate (reviewed, tested) label Jun 27, 2025
@pcd1193182
Copy link
Contributor Author

The math seems to make sense, but I am not sure I can make sense out of "don't subtract out parity for that row" comment. We are not just not subtracting parity, we ignore it at all.

True; the reason we don't subtract parity is the same as the reason we round down overall: because the row can't being used to store data. I can modify the comment to make that clearer.

When an allocation is happening on a raidz vdev, the number of sectors
allocated is rounded up to a multiple of nparity + 1. If this results in
the allocation spilling into an extra row, then the corresponding call
to vdev_raidz_asize_to_psize will incorrectly assume that parity sectors
were allocated for that spilled row, even though no data is stored
there.

If we determine that happened, we need to subtract out those extra
sectors before performing the rest of the capacity calculation.

Signed-off-by: Paul Dagnelie <[email protected]>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
@github-actions github-actions bot removed the Status: Accepted Ready to integrate (reviewed, tested) label Jun 27, 2025
@amotin amotin merged commit 69ee01a into openzfs:master Jun 27, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants