All of lore.kernel.org
 help / color / mirror / Atom feed
From: fdmanana@kernel.org
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 2/2] btrfs: fix data race at btrfs_use_block_rsv() when accessing block reserve
Date: Tue, 20 Feb 2024 12:24:34 +0000	[thread overview]
Message-ID: <2fb327ae5cb4c9d401a477de7c38918dd00aa538.1708429856.git.fdmanana@suse.com> (raw)
In-Reply-To: <cover.1708429856.git.fdmanana@suse.com>

From: Filipe Manana <fdmanana@suse.com>

At btrfs_use_block_rsv() we read the size of a block reserve without
locking its spinlock, which makes KCSAN complain because the size of a
block reserve is always updated while holding its spinlock. The report
from KCSAN is the following:

  [  653.313148] BUG: KCSAN: data-race in btrfs_update_delayed_refs_rsv [btrfs] / btrfs_use_block_rsv [btrfs]

  [  653.314755] read to 0x000000017f5871b8 of 8 bytes by task 7519 on cpu 0:
  [  653.314779]  btrfs_use_block_rsv+0xe4/0x2f8 [btrfs]
  [  653.315606]  btrfs_alloc_tree_block+0xdc/0x998 [btrfs]
  [  653.316421]  btrfs_force_cow_block+0x220/0xe38 [btrfs]
  [  653.317242]  btrfs_cow_block+0x1ac/0x568 [btrfs]
  [  653.318060]  btrfs_search_slot+0xda2/0x19b8 [btrfs]
  [  653.318879]  btrfs_del_csums+0x1dc/0x798 [btrfs]
  [  653.319702]  __btrfs_free_extent.isra.0+0xc24/0x2028 [btrfs]
  [  653.320538]  __btrfs_run_delayed_refs+0xd3c/0x2390 [btrfs]
  [  653.321340]  btrfs_run_delayed_refs+0xae/0x290 [btrfs]
  [  653.322140]  flush_space+0x5e4/0x718 [btrfs]
  [  653.322958]  btrfs_preempt_reclaim_metadata_space+0x102/0x2f8 [btrfs]
  [  653.323781]  process_one_work+0x3b6/0x838
  [  653.323800]  worker_thread+0x75e/0xb10
  [  653.323817]  kthread+0x21a/0x230
  [  653.323836]  __ret_from_fork+0x6c/0xb8
  [  653.323855]  ret_from_fork+0xa/0x30

  [  653.323887] write to 0x000000017f5871b8 of 8 bytes by task 576 on cpu 3:
  [  653.323906]  btrfs_update_delayed_refs_rsv+0x1a4/0x250 [btrfs]
  [  653.324699]  btrfs_add_delayed_data_ref+0x468/0x6d8 [btrfs]
  [  653.325494]  btrfs_free_extent+0x76/0x120 [btrfs]
  [  653.326280]  __btrfs_mod_ref+0x6a8/0x6b8 [btrfs]
  [  653.327064]  btrfs_dec_ref+0x50/0x70 [btrfs]
  [  653.327849]  walk_up_proc+0x236/0xa50 [btrfs]
  [  653.328633]  walk_up_tree+0x21c/0x448 [btrfs]
  [  653.329418]  btrfs_drop_snapshot+0x802/0x1328 [btrfs]
  [  653.330205]  btrfs_clean_one_deleted_snapshot+0x184/0x238 [btrfs]
  [  653.330995]  cleaner_kthread+0x2b0/0x2f0 [btrfs]
  [  653.331781]  kthread+0x21a/0x230
  [  653.331800]  __ret_from_fork+0x6c/0xb8
  [  653.331818]  ret_from_fork+0xa/0x30

So add a helper to get the size of a block reserve while holding the lock.
Reading the field while holding the lock instead of using the data_race()
annotation is used in order to prevent load tearing.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/block-rsv.c |  2 +-
 fs/btrfs/block-rsv.h | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 27207dad27c2..95c174f9fd4f 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -493,7 +493,7 @@ struct btrfs_block_rsv *btrfs_use_block_rsv(struct btrfs_trans_handle *trans,
 
 	block_rsv = get_block_rsv(trans, root);
 
-	if (unlikely(block_rsv->size == 0))
+	if (unlikely(btrfs_block_rsv_size(block_rsv) == 0))
 		goto try_reserve;
 again:
 	ret = btrfs_block_rsv_use_bytes(block_rsv, blocksize);
diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h
index 89c99f4e9f16..1f53b967d069 100644
--- a/fs/btrfs/block-rsv.h
+++ b/fs/btrfs/block-rsv.h
@@ -124,4 +124,20 @@ static inline u64 btrfs_block_rsv_reserved(struct btrfs_block_rsv *rsv)
 	return ret;
 }
 
+/*
+ * Get the size of a block reserve in a context where getting a stale value is
+ * acceptable, instead of accessing it directly and trigger data race warning
+ * from KCSAN.
+ */
+static inline u64 btrfs_block_rsv_size(struct btrfs_block_rsv *rsv)
+{
+	u64 ret;
+
+	spin_lock(&rsv->lock);
+	ret = rsv->size;
+	spin_unlock(&rsv->lock);
+
+	return ret;
+}
+
 #endif /* BTRFS_BLOCK_RSV_H */
-- 
2.40.1


  parent reply	other threads:[~2024-02-20 12:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-20 12:24 [PATCH 0/2] btrfs: fix a couple KCSAN warnings fdmanana
2024-02-20 12:24 ` [PATCH 1/2] btrfs: fix data races when accessing the reserved amount of block reserves fdmanana
2024-02-26 17:39   ` Sweet Tea Dorminy
2024-02-26 17:55     ` Filipe Manana
2024-02-20 12:24 ` fdmanana [this message]
2024-02-21 11:12 ` [PATCH 0/2] btrfs: fix a couple KCSAN warnings David Sterba
2024-02-21 11:25   ` Filipe Manana
2024-02-21 11:31     ` 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=2fb327ae5cb4c9d401a477de7c38918dd00aa538.1708429856.git.fdmanana@suse.com \
    --to=fdmanana@kernel.org \
    --cc=linux-btrfs@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.