Fix nonrot property being incorrectly unset #17206
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
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Signed-off-by: Paul Dagnelie <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc.
tonyhutter
approved these changes
Apr 1, 2025
amotin
approved these changes
Apr 2, 2025
robn
pushed a commit
to robn/zfs
that referenced
this pull request
Apr 4, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]> (cherry picked from commit 7be9fa2)
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Apr 8, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]>
fuporovvStack
pushed a commit
to fuporovvStack/zfs
that referenced
this pull request
Apr 11, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]>
fuporovvStack
pushed a commit
to fuporovvStack/zfs
that referenced
this pull request
Apr 11, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]>
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Apr 16, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]>
snajpa
pushed a commit
to vpsfreecz/zfs
that referenced
this pull request
May 2, 2025
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks. The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for. Sponsored-by: Klara, Inc. Sponsored-by: Eshtek, Inc. Signed-off-by: Paul Dagnelie <[email protected]> Co-authored-by: Paul Dagnelie <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Alexander Motin <[email protected]>
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.
Sponsored-by: Klara, Inc.
Sponsored-by: Eshtek, Inc.
Motivation and Context
When opening a vdev and setting the nonrot property, we used to wait for each child to be opened before examining its nonrot property. When the change was made to open vdevs asynchronously, we didn't move the nonrot check out of the main loop. As a result, the nonrot property is almost always set to false, regardless of the actual type of the underlying disks.
Description
The fix is simply to move the nonrot check to a separate loop after the taskq has been waited for.
How Has This Been Tested?
Ran through the test suite, which was failing the split tests with segment weighting disabled. Used zfs_dbgmsg to verify that nonrot was being set appropriately.
We should also consider augmenting the test suite to fuzz some tunables before/while running. This bug reproduces 100% of the time with segment weighting disabled, and it's almost certainly not the only bug of its type.
Types of changes
Checklist:
Signed-off-by.