Re: [PATCH v2 05/18] btrfs: do not allow free space tree rebuild on extent tree v2

David Sterba
2023-11-16T20:17:03Z

On Wed, Nov 15, 2023 at 05:49:48PM +0800, Anand Jain wrote:
> On 11/9/23 03:08, Josef Bacik wrote:
> > We currently don't allow these options to be set if we're extent tree v2
> > via the mount option parsing. However when we switch to the new mount
> > API we'll no longer have the super block loaded, so won't be able to
> > make this distinction at mount option parsing time. Address this by
> > checking for extent tree v2 at the point where we make the decision to
> > rebuild the free space tree.
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> > fs/btrfs/disk-io.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index b486cbec492b..072c45811c41 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -2951,7 +2951,8 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
> > bool rebuild_free_space_tree = false;
> >
> > if (btrfs_test_opt(fs_info, CLEAR_CACHE) &&
> > - btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
> > + btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> > + !btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
> > rebuild_free_space_tree = true;
> > } else if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> > !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE_VALID)) {
>
> If there is v3 you can consider to add a comment similar to that
> is in btrfs_parse_options().
> Also, IMO, it is a good idea to include a btrfs_info() statement
> to indicate that the clear_cache option is ignored.

Agreed, we have a lot of verbosity around the mount options, if some
option combination is invalid or not working as expected a message
should pe printed.