Skip to content

Commit

Permalink
Don't lock rt_lock in range_tree_verify()
Browse files Browse the repository at this point in the history
range_tree_verify() was the only range tree support function which
locked rt_lock whereas all the other functions required the lock to
be taken by the caller.  If the lock is taken in range_tree_verify(),
it's not possible to atomically verify a set of related range trees
(those which are likely protected by the same lock).

In the previous implementation, checking "related" trees would be done
as follows:

    range_tree_verify(tree1, offset, size);
    /* tree1's rt_lock is not taken here */
    range_tree_verify(tree2, offset, size);

The new implementation requires:

    mutex_enter(tree1->rt_lock);
    range_tree_verify(tree1, offset, size);
    range_tree_verify(tree2, offset, size);
    mutex_exit(tree1->rt_lock);

Currently, the only consumer of range_tree_verify() is
metaslab_check_free() which verifies a set of realted range trees in
a metaslab.  The TRIM/DISCARD code adds an additional set of checks of
the current and previous trimsets, both of which are represented as
range trees.

metaslab_check_free() has been updated to lock ms_lock once for each
vdev's metaslab and also for debugging builds to verify that the each
tree's rt_lock matches the metaslab's ms_lock to prove they're related.
  • Loading branch information
dweeezil committed Apr 8, 2017
1 parent 7448ab3 commit 46177f3
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 7 additions & 0 deletions module/zfs/metaslab.c
Original file line number Diff line number Diff line change
Expand Up @@ -3617,12 +3617,18 @@ metaslab_check_free(spa_t *spa, const blkptr_t *bp)
uint64_t size = DVA_GET_ASIZE(&bp->blk_dva[i]);
metaslab_t *msp = vd->vdev_ms[offset >> vd->vdev_ms_shift];

mutex_enter(&msp->ms_lock);
if (msp->ms_loaded) {
VERIFY(&msp->ms_lock == msp->ms_tree->rt_lock);
range_tree_verify(msp->ms_tree, offset, size);
#ifdef DEBUG
VERIFY(&msp->ms_lock ==
msp->ms_cur_ts->ts_tree->rt_lock);
range_tree_verify(msp->ms_cur_ts->ts_tree,
offset, size);
if (msp->ms_prev_ts != NULL) {
VERIFY(&msp->ms_lock ==
msp->ms_prev_ts->ts_tree->rt_lock);
range_tree_verify(msp->ms_prev_ts->ts_tree,
offset, size);
}
Expand All @@ -3633,6 +3639,7 @@ metaslab_check_free(spa_t *spa, const blkptr_t *bp)
range_tree_verify(msp->ms_freedtree, offset, size);
for (j = 0; j < TXG_DEFER_SIZE; j++)
range_tree_verify(msp->ms_defertree[j], offset, size);
mutex_exit(&msp->ms_lock);
}
spa_config_exit(spa, SCL_VDEV, FTAG);
}
Expand Down
2 changes: 0 additions & 2 deletions module/zfs/range_tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,9 @@ range_tree_verify(range_tree_t *rt, uint64_t off, uint64_t size)
{
range_seg_t *rs;

mutex_enter(rt->rt_lock);
rs = range_tree_find(rt, off, size);
if (rs != NULL)
panic("freeing free block; rs=%p", (void *)rs);
mutex_exit(rt->rt_lock);
}

boolean_t
Expand Down

0 comments on commit 46177f3

Please sign in to comment.