[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20241101035133.925251-1-zhenghaoran@buaa.edu.cn>
Date: Fri, 1 Nov 2024 11:51:33 +0800
From: Hao-ran Zheng <zhenghaoran@...a.edu.cn>
To: clm@...com,
josef@...icpanda.com,
dsterba@...e.com,
linux-btrfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: baijiaju1990@...il.com,
zhenghaoran@...a.edu.cn,
21371365@...a.edu.cn
Subject: [PATCH] btrfs: Fix data race in log_conflicting_inodes
The Data Race occurs when the `log_conflicting_inodes()` function is
executed in different threads at the same time. When one thread assigns
a value to `ctx->logging_conflict_inodes` while another thread performs
an `if(ctx->logging_conflict_inodes)` judgment or modifies it at the
same time, a data contention problem may arise.
Further, an atomicity violation may also occur here. Consider the
following case, when a thread A `if(ctx->logging_conflict_inodes)`
passes the judgment, the execution switches to another thread B, at
which time the value of `ctx->logging_conflict_inodes` has not yet
been assigned true, which would result in multiple threads executing
`log_conflicting_inodes()`.
To address this issue, it is recommended to add locks to protect
`logging_conflict_inodes` in the `btrfs_log_ctx` structure, and lock
protection during assignment and judgment. This modification ensures
that the value of `ctx->logging_conflict_inodes` does not change during
the validation process, thereby maintaining its integrity.
Signed-off-by: Hao-ran Zheng <zhenghaoran@...a.edu.cn>
---
fs/btrfs/tree-log.c | 7 +++++++
fs/btrfs/tree-log.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 9637c7cdc0cf..9cdbf280ca9a 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2854,6 +2854,7 @@ void btrfs_init_log_ctx(struct btrfs_log_ctx *ctx, struct btrfs_inode *inode)
INIT_LIST_HEAD(&ctx->conflict_inodes);
ctx->num_conflict_inodes = 0;
ctx->logging_conflict_inodes = false;
+ spin_lock_init(&ctx->logging_conflict_inodes_lock);
ctx->scratch_eb = NULL;
}
@@ -5779,16 +5780,20 @@ static int log_conflicting_inodes(struct btrfs_trans_handle *trans,
struct btrfs_log_ctx *ctx)
{
int ret = 0;
+ unsigned long logging_conflict_inodes_flags;
/*
* Conflicting inodes are logged by the first call to btrfs_log_inode(),
* otherwise we could have unbounded recursion of btrfs_log_inode()
* calls. This check guarantees we can have only 1 level of recursion.
*/
+ spin_lock_irqsave(&ctx->conflict_inodes_lock, logging_conflict_inodes_flags);
if (ctx->logging_conflict_inodes)
+ spin_unlock_irqrestore(&ctx->conflict_inodes_lock, logging_conflict_inodes_flags);
return 0;
ctx->logging_conflict_inodes = true;
+ spin_unlock_irqrestore(&ctx->conflict_inodes_lock, logging_conflict_inodes_flags);
/*
* New conflicting inodes may be found and added to the list while we
@@ -5869,7 +5874,9 @@ static int log_conflicting_inodes(struct btrfs_trans_handle *trans,
break;
}
+ spin_lock_irqsave(&ctx->conflict_inodes_lock, logging_conflict_inodes_flags);
ctx->logging_conflict_inodes = false;
+ spin_unlock_irqrestore(&ctx->conflict_inodes_lock, logging_conflict_inodes_flags);
if (ret)
free_conflicting_inodes(ctx);
diff --git a/fs/btrfs/tree-log.h b/fs/btrfs/tree-log.h
index dc313e6bb2fa..0f862d0c80f2 100644
--- a/fs/btrfs/tree-log.h
+++ b/fs/btrfs/tree-log.h
@@ -44,6 +44,7 @@ struct btrfs_log_ctx {
struct list_head conflict_inodes;
int num_conflict_inodes;
bool logging_conflict_inodes;
+ spinlock_t logging_conflict_inodes_lock;
/*
* Used for fsyncs that need to copy items from the subvolume tree to
* the log tree (full sync flag set or copy everything flag set) to
--
2.34.1
Powered by blists - more mailing lists