[PATCH] btrfs: annotate block group access with data_race() when sorting for reclaim

[PATCH] btrfs: annotate block group access with data_race() when sorting for reclaim

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When sorting the block group list for reclaim we are using a block group's
used bytes counter without taking the block group's spinlock, so we can
race with a concurrent task updating it (at btrfs_update_block_group()),
which makes tools like KCSAN unhappy and report a race.

Since the sorting is not strictly needed from a functional perspective
and such races should rarely cause any ordering changes (only load/store
tearing could cause them), not to mention that after the sorting the
ordering may no longer be accurate due to concurrent allocations and
deallocations of extents in a block group, annotate the accesses to the
used counter with data_race() to silence KCSAN and similar tools.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/block-group.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 239cbb01f83f..548483a84466 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1795,7 +1795,14 @@ static int reclaim_bgs_cmp(void *unused, const struct list_head *a,
 	bg1 = list_entry(a, struct btrfs_block_group, bg_list);
 	bg2 = list_entry(b, struct btrfs_block_group, bg_list);
 
-	return bg1->used > bg2->used;
+	/*
+	 * Some other task may be updating the ->used field concurrently, but it
+	 * is not serious if we get a stale value or load/store tearing issues,
+	 * as sorting the list of block groups to reclaim is not critical and an
+	 * occasional imperfect order is ok. So silence KCSAN and avoid the
+	 * overhead of locking or any other synchronization.
+	 */
+	return data_race(bg1->used > bg2->used);
 }
 
 static inline bool btrfs_should_reclaim(const struct btrfs_fs_info *fs_info)
-- 
2.47.2

Re: [PATCH] btrfs: annotate block group access with data_race() when sorting for reclaim

[Qu Wenruo]

在 2025/9/8 21:35, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When sorting the block group list for reclaim we are using a block group's
> used bytes counter without taking the block group's spinlock, so we can
> race with a concurrent task updating it (at btrfs_update_block_group()),
> which makes tools like KCSAN unhappy and report a race.
> 
> Since the sorting is not strictly needed from a functional perspective
> and such races should rarely cause any ordering changes (only load/store
> tearing could cause them), not to mention that after the sorting the
> ordering may no longer be accurate due to concurrent allocations and
> deallocations of extents in a block group, annotate the accesses to the
> used counter with data_race() to silence KCSAN and similar tools.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/block-group.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 239cbb01f83f..548483a84466 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1795,7 +1795,14 @@ static int reclaim_bgs_cmp(void *unused, const struct list_head *a,
>   	bg1 = list_entry(a, struct btrfs_block_group, bg_list);
>   	bg2 = list_entry(b, struct btrfs_block_group, bg_list);
>   
> -	return bg1->used > bg2->used;
> +	/*
> +	 * Some other task may be updating the ->used field concurrently, but it
> +	 * is not serious if we get a stale value or load/store tearing issues,
> +	 * as sorting the list of block groups to reclaim is not critical and an
> +	 * occasional imperfect order is ok. So silence KCSAN and avoid the
> +	 * overhead of locking or any other synchronization.
> +	 */
> +	return data_race(bg1->used > bg2->used);
>   }
>   
>   static inline bool btrfs_should_reclaim(const struct btrfs_fs_info *fs_info)

Re: [PATCH] btrfs: annotate block group access with data_race() when sorting for reclaim

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>