Skip to content

Commit 6f2080f

Browse files
amotintonyhutter
authored andcommitted
Fix lock reversal on device removal cancel
FreeBSD kernel's WITNESS code detected lock ordering violation in spa_vdev_remove_cancel_sync(). It took svr_lock while holding ms_lock, which is opposite to other places. I was thinking to resolve it similar to openzfs#17145, but looking closer I don't think we even need svr_lock at that point, since we already asserted svr_allocd_segs is empty, and we don't need to add there segments we are going to call free_mapped_segment_cb for. Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Allan Jude <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#17164
1 parent 9f0be8f commit 6f2080f

File tree

1 file changed

+22
-28
lines changed

1 file changed

+22
-28
lines changed

module/zfs/vdev_removal.c

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1894,6 +1894,8 @@ spa_vdev_remove_cancel_sync(void *arg, dmu_tx_t *tx)
18941894
vdev_indirect_mapping_max_offset(vim));
18951895
}
18961896

1897+
zfs_range_tree_t *segs = zfs_range_tree_create(NULL, ZFS_RANGE_SEG64,
1898+
NULL, 0, 0);
18971899
for (uint64_t msi = 0; msi < vd->vdev_ms_count; msi++) {
18981900
metaslab_t *msp = vd->vdev_ms[msi];
18991901

@@ -1913,38 +1915,30 @@ spa_vdev_remove_cancel_sync(void *arg, dmu_tx_t *tx)
19131915
ASSERT0(zfs_range_tree_space(msp->ms_defer[i]));
19141916
ASSERT0(zfs_range_tree_space(msp->ms_freed));
19151917

1916-
if (msp->ms_sm != NULL) {
1917-
mutex_enter(&svr->svr_lock);
1918-
VERIFY0(space_map_load(msp->ms_sm,
1919-
svr->svr_allocd_segs, SM_ALLOC));
1920-
1921-
zfs_range_tree_walk(msp->ms_unflushed_allocs,
1922-
zfs_range_tree_add, svr->svr_allocd_segs);
1923-
zfs_range_tree_walk(msp->ms_unflushed_frees,
1924-
zfs_range_tree_remove, svr->svr_allocd_segs);
1925-
zfs_range_tree_walk(msp->ms_freeing,
1926-
zfs_range_tree_remove, svr->svr_allocd_segs);
1927-
1928-
/*
1929-
* Clear everything past what has been synced,
1930-
* because we have not allocated mappings for it yet.
1931-
*/
1932-
uint64_t syncd = vdev_indirect_mapping_max_offset(vim);
1933-
uint64_t sm_end = msp->ms_sm->sm_start +
1934-
msp->ms_sm->sm_size;
1935-
if (sm_end > syncd)
1936-
zfs_range_tree_clear(svr->svr_allocd_segs,
1937-
syncd, sm_end - syncd);
1918+
if (msp->ms_sm != NULL)
1919+
VERIFY0(space_map_load(msp->ms_sm, segs, SM_ALLOC));
19381920

1939-
mutex_exit(&svr->svr_lock);
1940-
}
1921+
zfs_range_tree_walk(msp->ms_unflushed_allocs,
1922+
zfs_range_tree_add, segs);
1923+
zfs_range_tree_walk(msp->ms_unflushed_frees,
1924+
zfs_range_tree_remove, segs);
1925+
zfs_range_tree_walk(msp->ms_freeing,
1926+
zfs_range_tree_remove, segs);
19411927
mutex_exit(&msp->ms_lock);
19421928

1943-
mutex_enter(&svr->svr_lock);
1944-
zfs_range_tree_vacate(svr->svr_allocd_segs,
1945-
free_mapped_segment_cb, vd);
1946-
mutex_exit(&svr->svr_lock);
1929+
/*
1930+
* Clear everything past what has been synced,
1931+
* because we have not allocated mappings for it yet.
1932+
*/
1933+
uint64_t syncd = vdev_indirect_mapping_max_offset(vim);
1934+
uint64_t sm_end = msp->ms_sm->sm_start +
1935+
msp->ms_sm->sm_size;
1936+
if (sm_end > syncd)
1937+
zfs_range_tree_clear(segs, syncd, sm_end - syncd);
1938+
1939+
zfs_range_tree_vacate(segs, free_mapped_segment_cb, vd);
19471940
}
1941+
zfs_range_tree_destroy(segs);
19481942

19491943
/*
19501944
* Note: this must happen after we invoke free_mapped_segment_cb,

0 commit comments

Comments
 (0)