Re: Btrfs progs release 6.16.1

[Mark Harmstone <mark@harmstone.com>]

On 18/09/2025 1.17 pm, Filipe Manana wrote:
> On Thu, Sep 18, 2025 at 6:10 AM Mark Harmstone <mark@harmstone.com> wrote:
>>
>> On 18/09/2025 1.33 am, David Sterba wrote:
>>> On Wed, Sep 17, 2025 at 04:41:43PM +0100, Filipe Manana wrote:
>>>> With this btrfs-progs release, running 'btrfs check' fails on a
>>>> filesystem created by an older mkfs.btrfs.
>>>
>>>> A bisection points to:
>>>>
>>>> commit e2cf6a03796b73d446b086022c0dfcf6a6552928
>>>> Author: Mark Harmstone <maharmstone@fb.com>
>>>> Date:   Fri Jul 18 15:26:27 2025 +0100
>>>>
>>>>       btrfs-progs: use btrfs_lookup_block_group() in check_free_space_tree()
>>>
>>> Thanks for the report, I'll do a release with this patch reverted unless
>>> there's an updated fix.
>>
>> The patch is correct: older versions of mkfs were creating spurious
>> free-space entries. btrfs-check was supposed to diagnose this but there
>> was a bug that meant it only triggered for entries at the end of the tree.
> 
> Sure, but now we have btrfs check failing for every fs created up to
> btrfs-progs v16.0 since we have the free space tree feature.
> That makes scripts fail since btrfs check returns a non-zero status
> now for such fs.
> Do we have any code to remove those unnecessary and harmless free space entries?
> 
> This will confuse and potentially scare some users, besides breaking
> scripts that verify the result from btrfs check exit status.

To be clear, this patch does address an actual issue: it caused me problems
while developing btrfs-discard-check. Leo independently ran into it last year
too.

btrfs rescue clear-space-cache will get rid of the error, though it's
obviously a little heavy-handed. I don't know if btrfs check --repair fixes
the FST.

Maybe the thing to do is to add a compat_flag to say that the FST doesn't
contain spurious entries, and to clean these up and set the flag on mount
if it's not there - what do you think? Like we have for
BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID already, though compat rather
than compat_ro because the kernel won't create these entries itself.