]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
btrfs: remove redundant memory barrier from extent_io_tree_release()
authorFilipe Manana <fdmanana@suse.com>
Fri, 22 Sep 2023 10:39:05 +0000 (11:39 +0100)
committerDavid Sterba <dsterba@suse.com>
Thu, 12 Oct 2023 14:44:15 +0000 (16:44 +0200)
The memory barrier at extent_io_tree_release() is redundant. Holding
spin_lock here is not enough to drop the barrier completely.  We only
change the waitqueue of an extent state record while holding the tree
lock - see wait_on_state().

The update to waitqueue state will not become stale because there will
be an spin_unlock/spin_lock sequence between the change and waiting,
this implies a full memory barrier.

So remove the explicit smp_mb() barrier.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
[ reword reasoning ]
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/extent-io-tree.c

index 58b3ea36a727a0e02eab888a667d30a0f46b0352..19a7a0b3e8636d864a47ef32dd7e0bf96aecfad3 100644 (file)
@@ -115,12 +115,6 @@ void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 void extent_io_tree_release(struct extent_io_tree *tree)
 {
        spin_lock(&tree->lock);
-       /*
-        * Do a single barrier for the waitqueue_active check here, the state
-        * of the waitqueue should not change once extent_io_tree_release is
-        * called.
-        */
-       smp_mb();
        while (!RB_EMPTY_ROOT(&tree->state)) {
                struct rb_node *node;
                struct extent_state *state;
@@ -130,6 +124,11 @@ void extent_io_tree_release(struct extent_io_tree *tree)
                rb_erase(&state->rb_node, &tree->state);
                RB_CLEAR_NODE(&state->rb_node);
                ASSERT(!(state->state & EXTENT_LOCKED));
+               /*
+                * No need for a memory barrier here, as we are holding the tree
+                * lock and we only change the waitqueue while holding that lock
+                * (see wait_on_state()).
+                */
                ASSERT(!waitqueue_active(&state->wq));
                free_extent_state(state);