Re: [PATCH v2 0/2] btrfs-progs: mkfs free-space-info bug

[Boris Burkov <boris@bur.io>]

On Mon, Sep 30, 2024 at 03:23:10PM -0700, Leo Martins wrote:
> Currently mkfs creates a few block-groups to bootstrap the filesystem.
> Along with these block-groups some free-space-infos are created. When
> the block-group cleanup happens the block-groups are deleted and the
> free-space-infos are not. This patch set introduces a fix that deletes
> the free-space-infos when the block-groups are deleted.
> 
> When implementing a test for my changes I found that there was already
> some code in free-space-tree.c that attempts to verify that all
> free-space-infos correspond to a block-group. This code only checks if
> there exists a block-group that has an objectid >= free-space-info's
> objectid. I added an additional check to make sure that the block-group
> actually matches the free-space-info's objectid and offset.
> 
> Making this change to fsck will cause all filesystems that were created
> using mkfs.btrfs to warn that there is some free-space-info that doesn't
> correspond to a block-group. This seems like a bad idea, I considered
> adding to the message to signify that this is not a big issue and maybe
> point them to this patchset to get the fix. Open to ideas on how to
> handle this.

A few thoughts:
- You should also add a diagnostic to the in-kernel tree checker (but not
  one that flips us read-only)
- You can add support in check --repair for removing the free space info
- I don't think it hurts to use some softer language in the warning, but
  fundamentally people are going to be hitting it and dealing with it
  eventually, so better to do include it than not. Otherwise, it's just
  a timebomb until we write some kernel logic that DOES choke on this
  problem.

  I don't think there is a precedent for a very soft warning in fsck
  output, but if there is, let's just follow that. I certainly wouldn't
  hate links to a wiki, for example :)

Boris

> 
> Leo Martins (2):
>   btrfs-progs: delete free space when deleting block group
>   btrfs-progs: check free space maps to block group
> 
>  common/clear-cache.c            |  3 ++-
>  kernel-shared/extent-tree.c     | 10 ++++++++++
>  kernel-shared/free-space-tree.c |  3 +++
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> -- 
> 2.43.5
>