btrfs: add lockdep annotations for num_writers wait event

Annotate the num_writers wait event in fs/btrfs/transaction.c with
lockdep in order to catch deadlocks involving this wait event.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Ioannis Angelakopoulos <iangelak@fb.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index dfeb7174219ee0407c4dd0933517417d5c56051a..707e644bab929fabb958b5966d13b9fbd22c076e 100644 (file)
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1092,6 +1092,12 @@ struct btrfs_fs_info {
        /* Updates are not protected by any lock */
        struct btrfs_commit_stats commit_stats;
 
+       /*
+        * Annotations for transaction events (structures are empty when
+        * compiled without lockdep).
+        */
+       struct lockdep_map btrfs_trans_num_writers_map;
+
 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
        spinlock_t ref_verify_lock;
        struct rb_root block_tree;

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 2633137c3e9f1efc4d07bd262152104a3633e32a..a04b32f7df9d96a6f8805f8c6156cabbce7060fe 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2990,6 +2990,8 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
        mutex_init(&fs_info->zoned_data_reloc_io_lock);
        seqlock_init(&fs_info->profiles_lock);
 
+       btrfs_lockdep_init_map(fs_info, btrfs_trans_num_writers);
+
        INIT_LIST_HEAD(&fs_info->dirty_cowonly_roots);
        INIT_LIST_HEAD(&fs_info->space_info);
        INIT_LIST_HEAD(&fs_info->tree_mod_seq_list);

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 0bec10740ad3921ec878ed0368d9425a32843eac..b3cb54d852f85d644febbc78c516b0b632b59913 100644 (file)
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -313,6 +313,7 @@ loop:
                atomic_inc(&cur_trans->num_writers);
                extwriter_counter_inc(cur_trans, type);
                spin_unlock(&fs_info->trans_lock);
+               btrfs_lockdep_acquire(fs_info, btrfs_trans_num_writers);
                return 0;
        }
        spin_unlock(&fs_info->trans_lock);
@@ -334,16 +335,20 @@ loop:
        if (!cur_trans)
                return -ENOMEM;
 
+       btrfs_lockdep_acquire(fs_info, btrfs_trans_num_writers);
+
        spin_lock(&fs_info->trans_lock);
        if (fs_info->running_transaction) {
                /*
                 * someone started a transaction after we unlocked.  Make sure
                 * to redo the checks above
                 */
+               btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
                kfree(cur_trans);
                goto loop;
        } else if (BTRFS_FS_ERROR(fs_info)) {
                spin_unlock(&fs_info->trans_lock);
+               btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
                kfree(cur_trans);
                return -EROFS;
        }
@@ -1022,6 +1027,9 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
        extwriter_counter_dec(cur_trans, trans->type);
 
        cond_wake_up(&cur_trans->writer_wait);
+
+       btrfs_lockdep_release(info, btrfs_trans_num_writers);
+
        btrfs_put_transaction(cur_trans);
 
        if (current->journal_info == trans)
@@ -1994,6 +2002,12 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans, int err)
        if (cur_trans == fs_info->running_transaction) {
                cur_trans->state = TRANS_STATE_COMMIT_DOING;
                spin_unlock(&fs_info->trans_lock);
+
+               /*
+                * The thread has already released the lockdep map as reader
+                * already in btrfs_commit_transaction().
+                */
+               btrfs_might_wait_for_event(fs_info, btrfs_trans_num_writers);
                wait_event(cur_trans->writer_wait,
                           atomic_read(&cur_trans->num_writers) == 1);
 
@@ -2222,7 +2236,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
 
                        btrfs_put_transaction(prev_trans);
                        if (ret)
-                               goto cleanup_transaction;
+                               goto lockdep_release;
                } else {
                        spin_unlock(&fs_info->trans_lock);
                }
@@ -2236,7 +2250,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
                 */
                if (BTRFS_FS_ERROR(fs_info)) {
                        ret = -EROFS;
-                       goto cleanup_transaction;
+                       goto lockdep_release;
                }
        }
 
@@ -2250,19 +2264,21 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
 
        ret = btrfs_start_delalloc_flush(fs_info);
        if (ret)
-               goto cleanup_transaction;
+               goto lockdep_release;
 
        ret = btrfs_run_delayed_items(trans);
        if (ret)
-               goto cleanup_transaction;
+               goto lockdep_release;
 
        wait_event(cur_trans->writer_wait,
                   extwriter_counter_read(cur_trans) == 0);
 
        /* some pending stuffs might be added after the previous flush. */
        ret = btrfs_run_delayed_items(trans);
-       if (ret)
+       if (ret) {
+               btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
                goto cleanup_transaction;
+       }
 
        btrfs_wait_delalloc_flush(fs_info);
 
@@ -2284,6 +2300,14 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
        add_pending_snapshot(trans);
        cur_trans->state = TRANS_STATE_COMMIT_DOING;
        spin_unlock(&fs_info->trans_lock);
+
+       /*
+        * The thread has started/joined the transaction thus it holds the
+        * lockdep map as a reader. It has to release it before acquiring the
+        * lockdep map as a writer.
+        */
+       btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
+       btrfs_might_wait_for_event(fs_info, btrfs_trans_num_writers);
        wait_event(cur_trans->writer_wait,
                   atomic_read(&cur_trans->num_writers) == 1);
 
@@ -2515,6 +2539,10 @@ cleanup_transaction:
        cleanup_transaction(trans, ret);
 
        return ret;
+
+lockdep_release:
+       btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
+       goto cleanup_transaction;
 }
 
 /*