2021-04-07 07:13:00

by Jan Kara

Subject: [PATCH 2/2] ext4: Annotate data race in jbd2_journal_dirty_metadata()

Assertion checks in jbd2_journal_dirty_metadata() are known to be racy
but we don't want to be grabbing locks just for them. We thus recheck
them under b_state_lock only if it looks like they would fail. Annotate
the checks with data_race().

Reported-by: Hao Sun <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/jbd2/transaction.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 398d1d9209e2..e8fc45fd751f 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1479,8 +1479,8 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
* crucial to catch bugs so let's do a reliable check until the
* lockless handling is fully proven.
*/
- if (jh->b_transaction != transaction &&
- jh->b_next_transaction != transaction) {
+ if (data_race(jh->b_transaction != transaction &&
+ jh->b_next_transaction != transaction)) {
spin_lock(&jh->b_state_lock);
J_ASSERT_JH(jh, jh->b_transaction == transaction ||
jh->b_next_transaction == transaction);
@@ -1488,8 +1488,8 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
}
if (jh->b_modified == 1) {
/* If it's in our transaction it must be in BJ_Metadata list. */
- if (jh->b_transaction == transaction &&
- jh->b_jlist != BJ_Metadata) {
+ if (data_race(jh->b_transaction == transaction &&
+ jh->b_jlist != BJ_Metadata)) {
spin_lock(&jh->b_state_lock);
if (jh->b_transaction == transaction &&
jh->b_jlist != BJ_Metadata)
--
2.26.2

2021-04-10 01:25:02

by Theodore Ts'o

Subject: Re: [PATCH 2/2] ext4: Annotate data race in jbd2_journal_dirty_metadata()

On Tue, Apr 06, 2021 at 06:18:00PM +0200, Jan Kara wrote:
> Assertion checks in jbd2_journal_dirty_metadata() are known to be racy
> but we don't want to be grabbing locks just for them. We thus recheck
> them under b_state_lock only if it looks like they would fail. Annotate
> the checks with data_race().
>
> Reported-by: Hao Sun <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>

Thanks, applied.

- Ted