Re: [PATCH RFC v2] btrfs: keep mixed block group writable for relocation setup commit

[Qu Wenruo]

在 2026/8/22 00:44, Bartosz Chronowski 写道:
> On Fri, Aug 14, 2026 at 08:04:13AM +0930, Qu Wenruo wrote:
> > 在 2026/8/14 08:01, Qu Wenruo 写道:
> > > 在 2026/8/13 20:47, Bartosz Chronowski 写道:
> > > > Relocating a nearly full mixed block group can abort the filesystem
> > > > transaction with -ENOSPC and trigger a warning in cleanup_transaction().
> > > >
> > > > Making the mixed target read-only can lead to a condition where
> > > > metadata COW cannot use its free space. In particular,
> > > > btrfs_relocate_block_group() marks the mixed target read-only before
> > > > prepare_to_relocate() commits the setup transaction. find_free_extent()
> > > > then skips all free extents in the target. Commit-time COW still needs
> > > > new tree blocks, so the transaction can fail with -ENOSPC when no
> > > > suitable extent remains in another block group.
> > >
> > > Then why things like btrfs_inc_block_group_ro() fail with ENOSPC in the
> > > first place?
> >
> > Sorry, missing the important word "not".
> >
> > Why that function did *not* fail with ENOSPC in the first place?
> >
> > > I believe that's the root problem that your agent never explained.
>
> Because when it ran, it still showed enough space during the check.

That's the problem.

> The check in inc_block_group_ro() only compares the counters of that
> moment. For a mixed group it takes the data branch and tests:
>
>      space_info_used + target_available <= total_bytes

Because it only treats the bg as DATA, not the more strict METADATA.

For mixed bg, it should meet both the DATA and METADATA checks, not only 
the data one.

> The reproduction path showed values `3170304 + 122880 <= 9895936`.
> The check had 6602752 bytes to spare, so `btrfs_inc_block_group_ro()`
> returned 0. It reserved no physical metadata extent for the later setup work.
>
> The shortage appears after the switch. In a mixed group, data and
> metadata share the same space, so the read-only target also stops
> serving metadata. The setup commit in prepare_to_relocate() still has
> to COW tree blocks; its reservation is satisfied from the global
> block reserve, so nothing really fails until the physical allocation runs
> inside the commit.
> By then the rest of the pool was gone: the abort-time dump in the
> syzbot report shows used + pinned + reserved +
> readonly equal to the total, to the byte, with reservations still
> outstanding. find_free_extent() skipped the read-only target, found
> no usable extent elsewhere and no room for a new chunk, and a failed
> allocation inside a commit is a transaction abort.
>
> So btrfs_inc_block_group_ro() cannot report this ENOSPC: when it
> runs, the shortage does not exist yet. It is created between that
> check and the setup commit's first allocation, and nothing ties the
> early success to the later obligation.
>
> The expectation for the cleanest solution would be to that the function
> should simply have rejected this state.
> I looked for a condition-only change that does that, and
> I do not see a correct one - the recorded numbers rule it out.

Really? I found it shocking that you just missed the 
btrfs_can_overcommit() checks just lines below.

And stop throwing LLM word salad without going through your human brain.

I don't want to waste time arguing with an agent which can only stick to 
its flawed plan.

If you are not the person defending the design, but an agent, I find no 
value reviewing anything from you and your agent.