* [bug report] btrfs: periodic block_group reclaim
@ 2026-05-08 7:49 Dan Carpenter
2026-05-11 15:56 ` Boris Burkov
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2026-05-08 7:49 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs
Hello Boris Burkov,
Commit e4ca3932ae90 ("btrfs: periodic block_group reclaim") from Feb
2, 2024 (linux-next), leads to the following Smatch static checker
warning:
fs/btrfs/space-info.c:2139 do_reclaim_sweep()
warn: iterator 'bg' changed during iteration
fs/btrfs/space-info.c
2125 static bool do_reclaim_sweep(struct btrfs_space_info *space_info, int raid)
2126 {
2127 struct btrfs_block_group *bg;
2128 int thresh_pct;
2129 bool will_reclaim = false;
2130 bool urgent;
2131
2132 spin_lock(&space_info->lock);
2133 urgent = is_reclaim_urgent(space_info);
2134 thresh_pct = btrfs_calc_reclaim_threshold(space_info);
2135 spin_unlock(&space_info->lock);
2136
2137 down_read(&space_info->groups_sem);
2138 again:
--> 2139 list_for_each_entry(bg, &space_info->block_groups[raid], list) {
2140 u64 thresh;
2141 bool reclaim = false;
2142
2143 btrfs_get_block_group(bg);
We bump the refcount here.
2144 spin_lock(&bg->lock);
2145 thresh = mult_perc(bg->length, thresh_pct);
2146 if (bg->used < thresh && bg->reclaim_mark) {
2147 will_reclaim = true;
2148 reclaim = true;
2149 }
2150 bg->reclaim_mark++;
2151 spin_unlock(&bg->lock);
2152 if (reclaim)
2153 btrfs_mark_bg_to_reclaim(bg);
2154 btrfs_put_block_group(bg);
^^
This decrements the "bg" so now presumably a different thread could free
it. The race window between this btrfs_put_block_group() and the
btrfs_get_block_group() on the next iteration is pretty small.
Most likely this get/put pair could be removed. I can't see a reason
for it.
2155 }
2156
2157 /*
2158 * In situations where we are very motivated to reclaim (low unalloc)
2159 * use two passes to make the reclaim mark check best effort.
2160 *
2161 * If we have any staler groups, we don't touch the fresher ones, but if we
2162 * really need a block group, do take a fresh one.
2163 */
2164 if (!will_reclaim && urgent) {
2165 urgent = false;
2166 goto again;
2167 }
2168
2169 up_read(&space_info->groups_sem);
2170 return will_reclaim;
2171 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [bug report] btrfs: periodic block_group reclaim
2026-05-08 7:49 [bug report] btrfs: periodic block_group reclaim Dan Carpenter
@ 2026-05-11 15:56 ` Boris Burkov
0 siblings, 0 replies; 2+ messages in thread
From: Boris Burkov @ 2026-05-11 15:56 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-btrfs
On Fri, May 08, 2026 at 10:49:26AM +0300, Dan Carpenter wrote:
> Hello Boris Burkov,
>
> Commit e4ca3932ae90 ("btrfs: periodic block_group reclaim") from Feb
> 2, 2024 (linux-next), leads to the following Smatch static checker
> warning:
>
> fs/btrfs/space-info.c:2139 do_reclaim_sweep()
> warn: iterator 'bg' changed during iteration
>
> fs/btrfs/space-info.c
> 2125 static bool do_reclaim_sweep(struct btrfs_space_info *space_info, int raid)
> 2126 {
> 2127 struct btrfs_block_group *bg;
> 2128 int thresh_pct;
> 2129 bool will_reclaim = false;
> 2130 bool urgent;
> 2131
> 2132 spin_lock(&space_info->lock);
> 2133 urgent = is_reclaim_urgent(space_info);
> 2134 thresh_pct = btrfs_calc_reclaim_threshold(space_info);
> 2135 spin_unlock(&space_info->lock);
> 2136
> 2137 down_read(&space_info->groups_sem);
> 2138 again:
> --> 2139 list_for_each_entry(bg, &space_info->block_groups[raid], list) {
> 2140 u64 thresh;
> 2141 bool reclaim = false;
> 2142
> 2143 btrfs_get_block_group(bg);
>
> We bump the refcount here.
>
> 2144 spin_lock(&bg->lock);
> 2145 thresh = mult_perc(bg->length, thresh_pct);
> 2146 if (bg->used < thresh && bg->reclaim_mark) {
> 2147 will_reclaim = true;
> 2148 reclaim = true;
> 2149 }
> 2150 bg->reclaim_mark++;
> 2151 spin_unlock(&bg->lock);
> 2152 if (reclaim)
> 2153 btrfs_mark_bg_to_reclaim(bg);
> 2154 btrfs_put_block_group(bg);
> ^^
>
> This decrements the "bg" so now presumably a different thread could free
> it. The race window between this btrfs_put_block_group() and the
> btrfs_get_block_group() on the next iteration is pretty small.
> Most likely this get/put pair could be removed. I can't see a reason
> for it.
>
Hi,
This should be protected by read locking the groups_sem for the duration
of the loop. The code which unlinks block_group->list write locks
groups_sem and has a reference to the bg during that time.
There is at least one other loop that uses this same pattern
(find_free_extent() in fs/btrfs/extent-tree.c). I agree that it doesn't
make the most sense to get/put the refcount if it doesn't really protect
against anything and the rwsem is the actual protection.
There was a patch on the list a few months back that switched to a
per-cpu refcount for block_group->refs, which suggests that getting rid
of some of these logically redundant refcounts could matter for
performance too. I'll take a look soon.
Thanks,
Boris
> 2155 }
> 2156
> 2157 /*
> 2158 * In situations where we are very motivated to reclaim (low unalloc)
> 2159 * use two passes to make the reclaim mark check best effort.
> 2160 *
> 2161 * If we have any staler groups, we don't touch the fresher ones, but if we
> 2162 * really need a block group, do take a fresh one.
> 2163 */
> 2164 if (!will_reclaim && urgent) {
> 2165 urgent = false;
> 2166 goto again;
> 2167 }
> 2168
> 2169 up_read(&space_info->groups_sem);
> 2170 return will_reclaim;
> 2171 }
>
> This email is a free service from the Smatch-CI project [smatch.sf.net].
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-11 15:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 7:49 [bug report] btrfs: periodic block_group reclaim Dan Carpenter
2026-05-11 15:56 ` Boris Burkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox