Correct missing zil_claim() DTL updates #11218
Merged
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
ahrens
reviewed
Nov 18, 2020
behlendorf
force-pushed
the
issue-10942
branch
2 times, most recently
from
86447c2 to
35c2afc
Compare
|
Refreshed, comment updated. |
Commit 1d477c2 accidentally disabled DTL updates for the zil_claim()
case described at the end of vdev_stat_update() by unconditionally
disabling all DTL updates when loading. This was done to avoid
a deadlock on the vd_dtl_lock when loading the DTLs from disk.
The missing DTL updates can be restored by moving the space_map_load()
call outside the vd_dtl_lock. A private range tree is populated by
the reading space map and then merged in to the DTL_MISSING tree
under the lock.
vdev_dtl_contains <--- Takes vd->vd_dtl_lock
vdev_mirror_child_missing
vdev_mirror_io_start
zio_vdev_io_start
__zio_execute
arc_read
dbuf_issue_final_prefetch
dbuf_prefetch_impl
dbuf_prefetch
dmu_prefetch
space_map_iterate
space_map_load_length
space_map_load
vdev_dtl_load <--- Takes vd->vd_dtl_lock
vdev_load
spa_ld_load_vdev_metadata
spa_tryimport
Furthermore, the SPA_LOAD_NONE check in vdev_dtl_contains() leads to an
additional problem. Any resilvering which occurs before SPA_LOAD_NONE
is set will incorrectly determine that there's nothing to repair. This
can result in full redundancy not been restored for some blocks. In
the worst case scenario this can result in a non-importable pool.
Signed-off-by: Brian Behlendorf <[email protected]>
behlendorf
force-pushed
the
issue-10942
branch
from
35c2afc to
ab5a3a4
Compare
behlendorf
added
Status: Accepted
behlendorf
added a commit
that referenced
this pull request
Nov 22, 2020
Commit a1d477c accidentally disabled DTL updates for the zil_claim() case described at the end of vdev_stat_update() by unconditionally disabling all DTL updates when loading. This was done to avoid a deadlock on the vd_dtl_lock when loading the DTLs from disk. vdev_dtl_contains <--- Takes vd->vd_dtl_lock vdev_mirror_child_missing vdev_mirror_io_start zio_vdev_io_start __zio_execute arc_read dbuf_issue_final_prefetch dbuf_prefetch_impl dbuf_prefetch dmu_prefetch space_map_iterate space_map_load_length space_map_load vdev_dtl_load <--- Takes vd->vd_dtl_lock vdev_load spa_ld_load_vdev_metadata spa_tryimport The missing DTL updates can be restored by moving the space_map_load() call outside the vd_dtl_lock. A private range tree is populated by reading the space map and then merged in to the DTL_MISSING tree under the lock. Furthermore, the SPA_LOAD_NONE check in vdev_dtl_contains() leads to an additional problem. Any resilvering which occurs before SPA_LOAD_NONE is set will incorrectly determine that there's nothing to repair. This can result in full redundancy not being restored for some blocks. Reviewed-by: Matt Ahrens <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #11218
jsai20
pushed a commit
to jsai20/zfs
that referenced
this pull request
Mar 30, 2021
Commit a1d477c accidentally disabled DTL updates for the zil_claim() case described at the end of vdev_stat_update() by unconditionally disabling all DTL updates when loading. This was done to avoid a deadlock on the vd_dtl_lock when loading the DTLs from disk. vdev_dtl_contains <--- Takes vd->vd_dtl_lock vdev_mirror_child_missing vdev_mirror_io_start zio_vdev_io_start __zio_execute arc_read dbuf_issue_final_prefetch dbuf_prefetch_impl dbuf_prefetch dmu_prefetch space_map_iterate space_map_load_length space_map_load vdev_dtl_load <--- Takes vd->vd_dtl_lock vdev_load spa_ld_load_vdev_metadata spa_tryimport The missing DTL updates can be restored by moving the space_map_load() call outside the vd_dtl_lock. A private range tree is populated by reading the space map and then merged in to the DTL_MISSING tree under the lock. Furthermore, the SPA_LOAD_NONE check in vdev_dtl_contains() leads to an additional problem. Any resilvering which occurs before SPA_LOAD_NONE is set will incorrectly determine that there's nothing to repair. This can result in full redundancy not being restored for some blocks. Reviewed-by: Matt Ahrens <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#11218
sempervictus
pushed a commit
to sempervictus/zfs
that referenced
this pull request
May 31, 2021
Commit a1d477c accidentally disabled DTL updates for the zil_claim() case described at the end of vdev_stat_update() by unconditionally disabling all DTL updates when loading. This was done to avoid a deadlock on the vd_dtl_lock when loading the DTLs from disk. vdev_dtl_contains <--- Takes vd->vd_dtl_lock vdev_mirror_child_missing vdev_mirror_io_start zio_vdev_io_start __zio_execute arc_read dbuf_issue_final_prefetch dbuf_prefetch_impl dbuf_prefetch dmu_prefetch space_map_iterate space_map_load_length space_map_load vdev_dtl_load <--- Takes vd->vd_dtl_lock vdev_load spa_ld_load_vdev_metadata spa_tryimport The missing DTL updates can be restored by moving the space_map_load() call outside the vd_dtl_lock. A private range tree is populated by reading the space map and then merged in to the DTL_MISSING tree under the lock. Furthermore, the SPA_LOAD_NONE check in vdev_dtl_contains() leads to an additional problem. Any resilvering which occurs before SPA_LOAD_NONE is set will incorrectly determine that there's nothing to repair. This can result in full redundancy not being restored for some blocks. Reviewed-by: Matt Ahrens <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#11218
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
Issue #10942.
Description
Commit a1d477c accidentally disabled DTL updates for the zil_claim()
case described at the end of vdev_stat_update() by unconditionally
disabling all DTL updates when loading. This was done to avoid
a deadlock on the vd_dtl_lock when loading the DTLs from disk.
The missing DTL updates can be restored by moving the space_map_load()
call outside the vd_dtl_lock. A private range tree is populated by
the reading space map and then swapped in for the DTL_MISSING tree
under the lock.
How Has This Been Tested?
Locally using the reproducer provided in this #10942 (comment).
Without the patch applied I was able to consistently reproduce the issue
within a a few iterations (10-15 minutes). With it applied the reproducer
ran for 16 hours without any problem. Additionally,
ztestwas run forseveral hours to exercise the import path.
Types of changes
Checklist:
Signed-off-by.