Re: Endless "reclaiming chunk"/"relocating block group"

[Christoph Biedl <linux-kernel.bfrz@manchmal.in-ulm.de>]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

Zygo Blaxell wrote...

> TL;DR not enough people are testing new kernel code against old compilers.
> If it's problematic to keep the host system's gcc up to date, a build
> chroot for kernel building with an up to date toolchain is the way to go.

Indeed, but unfortunately this is not an option in the given environment.


Eventually I started bisecting and found the commit below introduced
the trouble. And just to make it clear, the change is correct as far as
I can see. The code built by gcc 5.4 is not.


commit ac2f1e63c65c695b6134f40a078cf82df627e188
Author: Josef Bacik <josef@toxicpanda.com>
Date:   Tue Mar 29 01:56:07 2022 -0700

    btrfs: allow block group background reclaim for non-zoned filesystems


These final three hunks caught my attention:


@@ -3220,6 +3245,8 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
        spin_unlock(&info->delalloc_root_lock);

        while (total) {
+               bool reclaim;
+
                cache = btrfs_lookup_block_group(info, bytenr);
                if (!cache) {
                        ret = -ENOENT;
@@ -3265,6 +3292,8 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
                                        cache->space_info, num_bytes);
                        cache->space_info->bytes_used -= num_bytes;
                        cache->space_info->disk_used -= num_bytes * factor;
+
+                       reclaim = should_reclaim_block_group(cache, num_bytes);
                        spin_unlock(&cache->lock);
                        spin_unlock(&cache->space_info->lock);

@@ -3291,6 +3320,8 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
                if (!alloc && old_val == 0) {
                        if (!btrfs_test_opt(info, DISCARD_ASYNC))
                                btrfs_mark_bg_unused(cache);
+               } else if (!alloc && reclaim) {
+                       btrfs_mark_bg_to_reclaim(cache);
                }

                btrfs_put_block_group(cache);


It seems strange "reclaim" is not initialized but after a closer look
this turns out to be okay. That variable is initialized if "alloc" is
false (not visible in the diff above), but only used under that
condition as well (last hunk). Which is why gcc does not emit warnings
about this.

*However*, some additional debug-print statements revealed the generated
code enters the block that calls btrfs_mark_bg_to_reclaim /even/ /if/
alloc is false, and reclaim undefined (usually true).

That is fairy scary, and I have no idea how many other places in the
kernel are affected by this. I'd assume, quite a few.

Workaround is trivial by initializing reclaim to false. But still.

    Christoph

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]