The reason why io_bgs can be modified without holding any lock is
non-obvious. Document it and reference that documentation from the
respective call sites

Signed-off-by: Nikolay Borisov <nbori...@suse.com>
---
 fs/btrfs/disk-io.c     |  4 ++++
 fs/btrfs/extent-tree.c |  7 ++++++-
 fs/btrfs/transaction.h | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1b3989c54d7c..b6fc734b0f5c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4342,6 +4342,10 @@ void btrfs_cleanup_dirty_bgs(struct btrfs_transaction 
*cur_trans,
        }
        spin_unlock(&cur_trans->dirty_bgs_lock);
 
+
+       /* Refer to the definition of io_bgs member for details why it's safe
+        * to use it without any locking
+        */
        while (!list_empty(&cur_trans->io_bgs)) {
                cache = list_first_entry(&cur_trans->io_bgs,
                                         struct btrfs_block_group_cache,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index cc08e6af3542..c4a3dfac224a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3742,7 +3742,8 @@ int btrfs_start_dirty_block_groups(struct 
btrfs_trans_handle *trans,
 
                                /*
                                 * the cache_write_mutex is protecting
-                                * the io_list
+                                * the io_list, also refer to the definition of
+                                * btrfs_transaction::io_bfs for more details
                                 */
                                list_add_tail(&cache->io_list, io);
                        } else {
@@ -3934,6 +3935,10 @@ int btrfs_write_dirty_block_groups(struct 
btrfs_trans_handle *trans,
        }
        spin_unlock(&cur_trans->dirty_bgs_lock);
 
+
+       /* Refer to the definition of io_bgs member for details why it's safe
+        * to use it without any locking
+        */
        while (!list_empty(io)) {
                cache = list_first_entry(io, struct btrfs_block_group_cache,
                                         io_list);
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 817fd7c9836b..90b80a6bea66 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -69,6 +69,21 @@ struct btrfs_transaction {
        struct list_head pending_chunks;
        struct list_head switch_commits;
        struct list_head dirty_bgs;
+
+       /* There is no explicit lock which protects io_bgs, rather its
+        * consistency is implied by the fact that all the sites which modify
+        * it do so under some form of transaction critical section, namely:
+        *
+        * * btrfs_start_dirty_block_groups - This function can only ever be
+        * run by one of the transaction committers. Refer to
+        * BTRFS_TRANS_DIRTY_BG_RUN usage in btrfs_commit_transaction
+        *
+        * * btrfs_write_dirty_blockgroups - this is called by
+        * commit_cowonly_roots from transaction critical section
+        * (TRANS_STATE_COMMIT_DOING)
+        *
+        * * btrfs_cleanup_dirty_bgs - called on transaction abort
+        */
        struct list_head io_bgs;
        struct list_head dropped_roots;
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to