public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Hao-ran Zheng <zhenghaoran154@gmail.com>
To: clm@fb.com, josef@toxicpanda.com, dsterba@suse.com,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: baijiaju1990@gmail.com, zhenghaoran154@gmail.com, 21371365@buaa.edu.cn
Subject: [PATCH] btrfs: fix data race when accessing the block_group's used field
Date: Sat,  8 Feb 2025 15:38:29 +0800	[thread overview]
Message-ID: <20250208073829.1176287-1-zhenghaoran154@gmail.com> (raw)

A data race may occur when the function `btrfs_discard_queue_work()`
and the function `btrfs_update_block_group()` is executed concurrently.
Specifically, when the `btrfs_update_block_group()` function is executed
to lines `cache->used = old_val;`, and `btrfs_discard_queue_work()`
is accessing `if(block_group->used == 0)` will appear with data race,
which may cause block_group to be placed unexpectedly in discard_list or
discard_unused_list. The specific function call stack is as follows:

============DATA_RACE============
 btrfs_discard_queue_work+0x245/0x500 [btrfs]
 __btrfs_add_free_space+0x3066/0x32f0 [btrfs]
 btrfs_add_free_space+0x19a/0x200 [btrfs]
 unpin_extent_range+0x847/0x2120 [btrfs]
 btrfs_finish_extent_commit+0x9a3/0x1840 [btrfs]
 btrfs_commit_transaction+0x5f65/0xc0f0 [btrfs]
 transaction_kthread+0x764/0xc20 [btrfs]
 kthread+0x292/0x330
 ret_from_fork+0x4d/0x80
 ret_from_fork_asm+0x1a/0x30
============OTHER_INFO============
 btrfs_update_block_group+0xa9d/0x2430 [btrfs]
 __btrfs_free_extent+0x4f69/0x9920 [btrfs]
 __btrfs_run_delayed_refs+0x290e/0xd7d0 [btrfs]
 btrfs_run_delayed_refs+0x317/0x770 [btrfs]
 flush_space+0x388/0x1440 [btrfs]
 btrfs_preempt_reclaim_metadata_space+0xd65/0x14c0 [btrfs]
 process_scheduled_works+0x716/0xf10
 worker_thread+0xb6a/0x1190
 kthread+0x292/0x330
 ret_from_fork+0x4d/0x80
 ret_from_fork_asm+0x1a/0x30
=================================

Although the `block_group->used` was checked again in the use of the
`peek_discard_list` function, considering that `block_group->used` is
a 64-bit variable, we still think that the data race here is an
unexpected behavior. It is recommended to add `READ_ONCE` and
`WRITE_ONCE` to read and write.

Signed-off-by: Hao-ran Zheng <zhenghaoran154@gmail.com>
---
 fs/btrfs/block-group.c | 4 ++--
 fs/btrfs/discard.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index c0a8f7d92acc..c681b97f6835 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3678,7 +3678,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 	old_val = cache->used;
 	if (alloc) {
 		old_val += num_bytes;
-		cache->used = old_val;
+		WRITE_ONCE(cache->used, old_val);
 		cache->reserved -= num_bytes;
 		cache->reclaim_mark = 0;
 		space_info->bytes_reserved -= num_bytes;
@@ -3690,7 +3690,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 		spin_unlock(&space_info->lock);
 	} else {
 		old_val -= num_bytes;
-		cache->used = old_val;
+		WRITE_ONCE(cache->used, old_val);
 		cache->pinned += num_bytes;
 		btrfs_space_info_update_bytes_pinned(space_info, num_bytes);
 		space_info->bytes_used -= num_bytes;
diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c
index e815d165cccc..71c57b571d50 100644
--- a/fs/btrfs/discard.c
+++ b/fs/btrfs/discard.c
@@ -363,7 +363,7 @@ void btrfs_discard_queue_work(struct btrfs_discard_ctl *discard_ctl,
 	if (!block_group || !btrfs_test_opt(block_group->fs_info, DISCARD_ASYNC))
 		return;
 
-	if (block_group->used == 0)
+	if (READ_ONCE(block_group->used) == 0)
 		add_to_discard_unused_list(discard_ctl, block_group);
 	else
 		add_to_discard_list(discard_ctl, block_group);
-- 
2.34.1


             reply	other threads:[~2025-02-08  7:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-08  7:38 Hao-ran Zheng [this message]
2025-02-10 11:09 ` [PATCH] btrfs: fix data race when accessing the block_group's used field Filipe Manana
2025-02-18  8:08   ` Daniel Vacek
2025-02-18 10:18     ` Filipe Manana
2025-02-18 16:13       ` Daniel Vacek
2025-02-18 16:28         ` Filipe Manana
2025-02-19  7:47           ` Daniel Vacek
2025-02-19  8:52           ` haoran zheng
2025-02-25 11:43             ` Filipe Manana
2025-02-26 15:00               ` 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=20250208073829.1176287-1-zhenghaoran154@gmail.com \
    --to=zhenghaoran154@gmail.com \
    --cc=21371365@buaa.edu.cn \
    --cc=baijiaju1990@gmail.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@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