[PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure

Daniel Vacek posted 8 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure
Posted by Daniel Vacek 2 months, 4 weeks ago
From: Josef Bacik <josef@toxicpanda.com>

btrfs/330 uncovered a problem where we were accidentally turning off the
free space tree when we do the transition from ro->rw.  This happens
because we don't update

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
Since v5 just a rebase conflict with changing API of mount_opt flags
being 64bit now.
---
 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/super.c   | 28 +++++++++++++++-------------
 fs/btrfs/super.h   |  3 ++-
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6a1fa3b08b3f..3bc7a773b900 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3399,7 +3399,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	 * Handle the space caching options appropriately now that we have the
 	 * super block loaded and validated.
 	 */
-	btrfs_set_free_space_cache_settings(fs_info);
+	btrfs_set_free_space_cache_settings(fs_info, &fs_info->mount_opt);
 
 	if (!btrfs_check_options(fs_info, &fs_info->mount_opt, sb->s_flags)) {
 		ret = -EINVAL;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4ffd7059e27a..f8759b856174 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -723,10 +723,10 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
 }
 
 /*
- * This is subtle, we only call this during open_ctree().  We need to pre-load
- * the mount options with the on-disk settings.  Before the new mount API took
- * effect we would do this on mount and remount.  With the new mount API we'll
- * only do this on the initial mount.
+ * Because we have an odd set of behavior with turning on and off the space cache
+ * and free space tree we have to call this before we start the mount operation
+ * after we load the super, or before we start remount.  This is to make sure we
+ * have the proper free space settings in place if the user didn't specify any.
  *
  * This isn't a change in behavior, because we're using the current state of the
  * file system to set the current mount options.  If you mounted with special
@@ -734,15 +734,16 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
  * settings, because mounting without these features cleared the on-disk
  * settings, so this being called on re-mount is not needed.
  */
-void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
+void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info,
+					 unsigned long long *mount_opt)
 {
 	if (fs_info->sectorsize < PAGE_SIZE) {
-		btrfs_clear_opt(fs_info->mount_opt, SPACE_CACHE);
-		if (!btrfs_test_opt(fs_info, FREE_SPACE_TREE)) {
+		btrfs_clear_opt(*mount_opt, SPACE_CACHE);
+		if (!btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE)) {
 			btrfs_info(fs_info,
 				   "forcing free space tree for sector size %u with page size %lu",
 				   fs_info->sectorsize, PAGE_SIZE);
-			btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
+			btrfs_set_opt(*mount_opt, FREE_SPACE_TREE);
 		}
 	}
 
@@ -750,7 +751,7 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 	 * At this point our mount options are populated, so we only mess with
 	 * these settings if we don't have any settings already.
 	 */
-	if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
+	if (btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE))
 		return;
 
 	if (btrfs_is_zoned(fs_info) &&
@@ -760,10 +761,10 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 		return;
 	}
 
-	if (btrfs_test_opt(fs_info, SPACE_CACHE))
+	if (btrfs_raw_test_opt(*mount_opt, SPACE_CACHE))
 		return;
 
-	if (btrfs_test_opt(fs_info, NOSPACECACHE))
+	if (btrfs_raw_test_opt(*mount_opt, NOSPACECACHE))
 		return;
 
 	/*
@@ -771,9 +772,9 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 	 * them ourselves based on the state of the file system.
 	 */
 	if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
-		btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
+		btrfs_set_opt(*mount_opt, FREE_SPACE_TREE);
 	else if (btrfs_free_space_cache_v1_active(fs_info))
-		btrfs_set_opt(fs_info->mount_opt, SPACE_CACHE);
+		btrfs_set_opt(*mount_opt, SPACE_CACHE);
 }
 
 static void set_device_specific_options(struct btrfs_fs_info *fs_info)
@@ -1523,6 +1524,7 @@ static int btrfs_reconfigure(struct fs_context *fc)
 
 	sync_filesystem(sb);
 	set_bit(BTRFS_FS_STATE_REMOUNTING, &fs_info->fs_state);
+	btrfs_set_free_space_cache_settings(fs_info, &ctx->mount_opt);
 
 	if (!btrfs_check_options(fs_info, &ctx->mount_opt, fc->sb_flags))
 		return -EINVAL;
diff --git a/fs/btrfs/super.h b/fs/btrfs/super.h
index d80a86acfbbe..584f428d36e2 100644
--- a/fs/btrfs/super.h
+++ b/fs/btrfs/super.h
@@ -16,7 +16,8 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
 int btrfs_sync_fs(struct super_block *sb, int wait);
 char *btrfs_get_subvol_name_from_objectid(struct btrfs_fs_info *fs_info,
 					  u64 subvol_objectid);
-void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info);
+void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info,
+					 unsigned long long *mount_opt);
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
 {
-- 
2.51.0
Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure
Posted by David Sterba 2 months, 3 weeks ago
On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> btrfs/330 uncovered a problem where we were accidentally turning off the
> free space tree when we do the transition from ro->rw.  This happens
> because we don't update

Missing text.
Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure
Posted by Daniel Vacek 2 months, 3 weeks ago
On Thu, 13 Nov 2025 at 11:32, David Sterba <dsterba@suse.cz> wrote:
> On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > btrfs/330 uncovered a problem where we were accidentally turning off the
> > free space tree when we do the transition from ro->rw.  This happens
> > because we don't update
>
> Missing text.

Hmm, this patch is new to v5. It doesn't even look encryption related.
I have no idea what Josef really means here.

The whole idea seems to be to call
btrfs_set_free_space_cache_settings() from btrfs_reconfigure() and to
update the ctx->mount_opt instead of fs_info->mount_opt while
remounting.

And btrfs/330 is not failing even with the full patchset applied
without this patch. I'm wondering if it is still needed after those
years?
Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure
Posted by David Sterba 2 months, 3 weeks ago
On Thu, Nov 13, 2025 at 12:24:33PM +0100, Daniel Vacek wrote:
> On Thu, 13 Nov 2025 at 11:32, David Sterba <dsterba@suse.cz> wrote:
> > On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> > > From: Josef Bacik <josef@toxicpanda.com>
> > >
> > > btrfs/330 uncovered a problem where we were accidentally turning off the
> > > free space tree when we do the transition from ro->rw.  This happens
> > > because we don't update
> >
> > Missing text.
> 
> Hmm, this patch is new to v5. It doesn't even look encryption related.
> I have no idea what Josef really means here.
> 
> The whole idea seems to be to call
> btrfs_set_free_space_cache_settings() from btrfs_reconfigure() and to
> update the ctx->mount_opt instead of fs_info->mount_opt while
> remounting.
> 
> And btrfs/330 is not failing even with the full patchset applied
> without this patch. I'm wondering if it is still needed after those
> years?

Maybe it it's not, as all the patches are somehow independent you can
drop it for now. We can add it later if need be.