From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Ioannis Angelakopoulos <iangelak@fb.com>,
Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>, Sasha Levin <sashal@kernel.org>,
clm@fb.com, linux-btrfs@vger.kernel.org
Subject: [PATCH AUTOSEL 6.0 25/46] btrfs: add lockdep annotations for num_writers wait event
Date: Tue, 11 Oct 2022 10:49:53 -0400 [thread overview]
Message-ID: <20221011145015.1622882-25-sashal@kernel.org> (raw)
In-Reply-To: <20221011145015.1622882-1-sashal@kernel.org>
From: Ioannis Angelakopoulos <iangelak@fb.com>
[ Upstream commit e1489b4fe6045a79a5e9c658eed65311977e230a ]
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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/btrfs/ctree.h | 6 ++++++
fs/btrfs/disk-io.c | 2 ++
fs/btrfs/transaction.c | 38 +++++++++++++++++++++++++++++++++-----
3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index dfeb7174219e..707e644bab92 100644
--- 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 2633137c3e9f..a04b32f7df9d 100644
--- 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 0bec10740ad3..b3cb54d852f8 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -313,6 +313,7 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,
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 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,
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 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
cleanup_transaction(trans, ret);
return ret;
+
+lockdep_release:
+ btrfs_lockdep_release(fs_info, btrfs_trans_num_writers);
+ goto cleanup_transaction;
}
/*
--
2.35.1
next prev parent reply other threads:[~2022-10-11 14:52 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20221011145015.1622882-1-sashal@kernel.org>
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 23/46] btrfs: dump extra info if one free space cache has more bitmaps than it should Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 24/46] btrfs: add macros for annotating wait events with lockdep Sasha Levin
2022-10-11 14:49 ` Sasha Levin [this message]
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 26/46] btrfs: add lockdep annotations for num_extwriters wait event Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 27/46] btrfs: add lockdep annotations for transaction states wait events Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 28/46] btrfs: add lockdep annotations for pending_ordered wait event Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 29/46] btrfs: change the lockdep class of free space inode's invalidate_lock Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 30/46] btrfs: add lockdep annotations for the ordered extents wait event Sasha Levin
2022-10-11 14:49 ` [PATCH AUTOSEL 6.0 31/46] btrfs: scrub: properly report super block errors in system log Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 32/46] btrfs: scrub: try to fix super block errors Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 33/46] btrfs: get rid of block group caching progress logic Sasha Levin
2022-10-11 23:46 ` Omar Sandoval
2022-10-13 17:55 ` Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 34/46] btrfs: don't print information about space cache or tree every remount Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 35/46] btrfs: call __btrfs_remove_free_space_cache_locked on cache load failure Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 36/46] btrfs: check superblock to ensure the fs was not modified at thaw time Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 37/46] btrfs: remove the unnecessary result variables Sasha Levin
2022-10-12 11:54 ` David Sterba
2022-10-13 17:56 ` Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 38/46] btrfs: introduce BTRFS_QGROUP_RUNTIME_FLAG_CANCEL_RESCAN Sasha Levin
2022-10-12 12:56 ` David Sterba
2022-10-12 23:12 ` Qu Wenruo
2022-10-13 17:56 ` Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 39/46] btrfs: add KCSAN annotations for unlocked access to block_rsv->full Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 40/46] btrfs: separate out the eb and extent state leak helpers Sasha Levin
2022-10-11 14:50 ` [PATCH AUTOSEL 6.0 41/46] btrfs: relax block-group-tree feature dependency checks Sasha Levin
2022-10-12 13:01 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221011145015.1622882-25-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=iangelak@fb.com \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).