[bug report] btrfs: periodic block_group reclaim

[bug report] btrfs: periodic block_group reclaim

[Dan Carpenter]

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