btrfs: rename cache_write_mutex to dirty_bgs_update_mutex

The v1 space cache writeout is gone, but the mutex is still needed. It
keeps btrfs_remove_block_group() from deleting a block group item while
btrfs_start_dirty_block_groups() is updating it outside the commit
critical section.

Rename it to reflect what it protects, and update the comments around
the dirty block group writeout that still refer to the space cache.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 03d5779358ca2973e74729a8bd59948ab0c1e053..228ceea58b10ece39a3d27d3350b6fd8c22eb32c 100644 (file)
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1196,7 +1196,11 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
 
        inode = lookup_free_space_inode(block_group, path);
 
-       mutex_lock(&trans->transaction->cache_write_mutex);
+       /*
+        * Do not delete the block group item while
+        * btrfs_start_dirty_block_groups() is updating it.
+        */
+       mutex_lock(&trans->transaction->dirty_bgs_update_mutex);
        spin_lock(&trans->transaction->dirty_bgs_lock);
        if (!list_empty(&block_group->dirty_list)) {
                list_del_init(&block_group->dirty_list);
@@ -1204,7 +1208,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
                btrfs_put_block_group(block_group);
        }
        spin_unlock(&trans->transaction->dirty_bgs_lock);
-       mutex_unlock(&trans->transaction->cache_write_mutex);
+       mutex_unlock(&trans->transaction->dirty_bgs_update_mutex);
 
        ret = btrfs_remove_free_space_inode(trans, inode, block_group);
        if (unlikely(ret)) {
@@ -3358,15 +3362,15 @@ fail:
 }
 
 /*
- * Transaction commit does final block group cache writeback during a critical
+ * Transaction commit does the final block group item updates during a critical
  * section where nothing is allowed to change the FS.  This is required in
- * order for the cache to actually match the block group, but can introduce a
+ * order for the items to actually match the block groups, but can introduce a
  * lot of latency into the commit.
  *
- * So, btrfs_start_dirty_block_groups is here to kick off block group cache IO.
- * There's a chance we'll have to redo some of it if the block group changes
- * again during the commit, but it greatly reduces the commit latency by
- * getting rid of the easy block groups while we're still allowing others to
+ * So, btrfs_start_dirty_block_groups is here to update the block group items
+ * early.  There's a chance we'll have to redo some of it if the block group
+ * changes again during the commit, but it greatly reduces the commit latency
+ * by getting rid of the easy block groups while we're still allowing others to
  * join the commit.
  */
 int btrfs_start_dirty_block_groups(struct btrfs_trans_handle *trans)
@@ -3400,11 +3404,11 @@ again:
        }
 
        /*
-        * cache_write_mutex is here only to save us from balance or automatic
-        * removal of empty block groups deleting this block group while we are
-        * updating its item
+        * dirty_bgs_update_mutex is here only to save us from balance or
+        * automatic removal of empty block groups deleting this block group
+        * while we are updating its item
         */
-       mutex_lock(&trans->transaction->cache_write_mutex);
+       mutex_lock(&trans->transaction->dirty_bgs_update_mutex);
        while (!list_empty(&dirty)) {
                bool drop_reserve = true;
 
@@ -3446,12 +3450,12 @@ again:
                if (drop_reserve)
                        btrfs_dec_delayed_refs_rsv_bg_updates(fs_info);
                /* Avoid blocking other tasks for too long. */
-               mutex_unlock(&trans->transaction->cache_write_mutex);
+               mutex_unlock(&trans->transaction->dirty_bgs_update_mutex);
                if (ret)
                        goto out;
-               mutex_lock(&trans->transaction->cache_write_mutex);
+               mutex_lock(&trans->transaction->dirty_bgs_update_mutex);
        }
-       mutex_unlock(&trans->transaction->cache_write_mutex);
+       mutex_unlock(&trans->transaction->dirty_bgs_update_mutex);
 
        /*
         * Go through delayed refs for all the stuff we've just kicked off
@@ -3465,7 +3469,7 @@ again:
                list_splice_init(&cur_trans->dirty_bgs, &dirty);
                /*
                 * dirty_bgs_lock protects us from concurrent block group
-                * deletes too (not just cache_write_mutex).
+                * deletes too (not just dirty_bgs_update_mutex).
                 */
                if (!list_empty(&dirty)) {
                        spin_unlock(&cur_trans->dirty_bgs_lock);
@@ -3561,10 +3565,10 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
        factor = btrfs_bg_type_to_factor(cache->flags);
 
        /*
-        * If this block group has free space cache written out, we need to make
-        * sure to load it if we are removing space.  This is because we need
-        * the unpinning stage to actually add the space back to the block group,
-        * otherwise we will leak space.
+        * Make sure the free space of this block group is loaded if we are
+        * removing space.  This is because we need the unpinning stage to
+        * actually add the space back to the block group, otherwise we will
+        * leak space.
         */
        if (!alloc && !btrfs_block_group_done(cache))
                btrfs_cache_block_group(cache, true);
@@ -3620,7 +3624,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
        /*
         * No longer have used bytes in this block group, queue it for deletion.
         * We do this after adding the block group to the dirty list to avoid
-        * races between cleaner kthread and space cache writeout.
+        * races between the cleaner kthread and the dirty block group writeout.
         */
        if (!alloc && old_val == 0) {
                if (!btrfs_test_opt(info, DISCARD_ASYNC))

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index a5097714578b2af6bd95510e5a513a77d48c2ccd..61fe4a889c9b6faa3641dd3ede5a1958523d8054 100644 (file)
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -380,7 +380,7 @@ loop:
        INIT_LIST_HEAD(&cur_trans->switch_commits);
        INIT_LIST_HEAD(&cur_trans->dirty_bgs);
        INIT_LIST_HEAD(&cur_trans->dropped_roots);
-       mutex_init(&cur_trans->cache_write_mutex);
+       mutex_init(&cur_trans->dirty_bgs_update_mutex);
        spin_lock_init(&cur_trans->dirty_bgs_lock);
        INIT_LIST_HEAD(&cur_trans->deleted_bgs);
        spin_lock_init(&cur_trans->dropped_roots_lock);
@@ -2267,18 +2267,16 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
        if (!test_bit(BTRFS_TRANS_DIRTY_BG_RUN, &cur_trans->flags)) {
                bool run_it = false;
 
-               /* this mutex is also taken before trying to set
-                * block groups readonly.  We need to make sure
-                * that nobody has set a block group readonly
-                * after a extents from that block group have been
-                * allocated for cache files.  btrfs_set_block_group_ro
-                * will wait for the transaction to commit if it
-                * finds BTRFS_TRANS_DIRTY_BG_RUN set.
+               /*
+                * This mutex is also taken before trying to set block groups
+                * readonly.  btrfs_inc_block_group_ro() will wait for the
+                * transaction to commit if it finds BTRFS_TRANS_DIRTY_BG_RUN
+                * set.
                 *
                 * The BTRFS_TRANS_DIRTY_BG_RUN flag is also used to make sure
-                * only one process starts all the block group IO.  It wouldn't
-                * hurt to have more than one go through, but there's no
-                * real advantage to it either.
+                * only one process starts all the block group item updates.  It
+                * wouldn't hurt to have more than one go through, but there's
+                * no real advantage to it either.
                 */
                mutex_lock(&fs_info->ro_block_group_mutex);
                if (!test_and_set_bit(BTRFS_TRANS_DIRTY_BG_RUN,
@@ -2512,10 +2510,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
        if (unlikely(ret))
                goto unlock_reloc;
 
-       /*
-        * The tasks which save the space cache and inode cache may also
-        * update ->aborted, check it.
-        */
+       /* Other tasks may also have updated ->aborted, check it. */
        if (TRANS_ABORTED(cur_trans)) {
                ret = cur_trans->aborted;
                goto unlock_reloc;

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 24b9be1833afb1710a46c555e60ab7189c0f160b..17d136675d49db89e5e1473c6d00d3ea9f00933e 100644 (file)
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -81,11 +81,11 @@ struct btrfs_transaction {
        struct extent_io_tree pinned_extents;
 
        /*
-        * we need to make sure block group deletion doesn't race with
-        * free space cache writeout.  This mutex keeps them from stomping
-        * on each other
+        * We need to make sure block group deletion doesn't race with the
+        * dirty block group item updates done outside the commit critical
+        * section. This mutex keeps them from stomping on each other.
         */
-       struct mutex cache_write_mutex;
+       struct mutex dirty_bgs_update_mutex;
        spinlock_t dirty_bgs_lock;
        /* Protected by spin lock fs_info->unused_bgs_lock. */
        struct list_head deleted_bgs;