| Message ID | 29dfd7c6aebbfbd0638a2e34ae34aa1d4f550389.1693209858.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: updates to error path for delayed dir index insertion failure | expand |
On 2023/8/28 16:06, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When removing a delayed item, or releasing which will remove it as well, > we will modify one of the delayed node's rbtrees and item counter if the > delayed item is in one of the rbtrees. This require having the delayed > node's mutex locked, otherwise we will race with other tasks modifying > the rbtrees and the counter. > > This is motivated by a previous version of another patch actually calling > btrfs_release_delayed_item() after unlocking the delayed node's mutex and > against a delayed item that is in a rbtree. > > So assert at __btrfs_remove_delayed_item() that the delayed node's mutex > is locked. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Although it would be even better to add lockdep asserts for both __btrfs_add_delayed_item() and __btrfs_remove_delayed_item(). Thanks, Qu > --- > fs/btrfs/delayed-inode.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > index eb175ae52245..8534285f760d 100644 > --- a/fs/btrfs/delayed-inode.c > +++ b/fs/btrfs/delayed-inode.c > @@ -412,6 +412,7 @@ static void finish_one_item(struct btrfs_delayed_root *delayed_root) > > static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item) > { > + struct btrfs_delayed_node *delayed_node = delayed_item->delayed_node; > struct rb_root_cached *root; > struct btrfs_delayed_root *delayed_root; > > @@ -419,18 +420,21 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item) > if (RB_EMPTY_NODE(&delayed_item->rb_node)) > return; > > - delayed_root = delayed_item->delayed_node->root->fs_info->delayed_root; > + /* If it's in a rbtree, then we need to have delayed node locked. */ > + lockdep_assert_held(&delayed_node->mutex); > + > + delayed_root = delayed_node->root->fs_info->delayed_root; > > BUG_ON(!delayed_root); > > if (delayed_item->type == BTRFS_DELAYED_INSERTION_ITEM) > - root = &delayed_item->delayed_node->ins_root; > + root = &delayed_node->ins_root; > else > - root = &delayed_item->delayed_node->del_root; > + root = &delayed_node->del_root; > > rb_erase_cached(&delayed_item->rb_node, root); > RB_CLEAR_NODE(&delayed_item->rb_node); > - delayed_item->delayed_node->count--; > + delayed_node->count--; > > finish_one_item(delayed_root); > }
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index eb175ae52245..8534285f760d 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -412,6 +412,7 @@ static void finish_one_item(struct btrfs_delayed_root *delayed_root) static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item) { + struct btrfs_delayed_node *delayed_node = delayed_item->delayed_node; struct rb_root_cached *root; struct btrfs_delayed_root *delayed_root; @@ -419,18 +420,21 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item) if (RB_EMPTY_NODE(&delayed_item->rb_node)) return; - delayed_root = delayed_item->delayed_node->root->fs_info->delayed_root; + /* If it's in a rbtree, then we need to have delayed node locked. */ + lockdep_assert_held(&delayed_node->mutex); + + delayed_root = delayed_node->root->fs_info->delayed_root; BUG_ON(!delayed_root); if (delayed_item->type == BTRFS_DELAYED_INSERTION_ITEM) - root = &delayed_item->delayed_node->ins_root; + root = &delayed_node->ins_root; else - root = &delayed_item->delayed_node->del_root; + root = &delayed_node->del_root; rb_erase_cached(&delayed_item->rb_node, root); RB_CLEAR_NODE(&delayed_item->rb_node); - delayed_item->delayed_node->count--; + delayed_node->count--; finish_one_item(delayed_root); }