| Message ID | 6a2c827b0ed8b24c3be1045ccac49b29e850118e.1699470345.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: convert to the new mount API | expand |
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. -------------------- case Opt_clear_cache: /* * We cannot clear the free space tree with extent tree * v2, ignore this option. */ if (btrfs_fs_incompat(info, EXTENT_TREE_V2)) break; btrfs_set_and_info(info, CLEAR_CACHE, "force clearing of disk cache"); break; -------------------- Thanks, Anand
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.
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)) {
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(-)