| Message ID | 1d0cca6ce1f67484c6b7ef591e264c04ca740c96.1600282812.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: free space tree mounting fixes | expand |
On 9/17/20 2:13 PM, Boris Burkov wrote: > When a user attempts to remount a btrfs filesystem with > 'mount -o remount,space_cache=v2', that operation silently succeeds. > Unfortunately, this is misleading, because the remount does not create > the free space tree. /proc/mounts will incorrectly show space_cache=v2, > but on the next mount, the file system will revert to the old > space_cache. > > For now, we handle only the easier case, where the existing mount is > read-only and the new mount is read-write. In that case, we can create > the free space tree without contending with the block groups changing > as we go. If the remount is ro->ro, rw->ro, or rw->rw, we will not > create the free space tree, and print a warning to dmesg so that this > failure is more visible. > > References: https://github.com/btrfs/btrfs-todo/issues/5 > Signed-off-by: Boris Burkov <boris@bur.io> Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On Thu, Sep 17, 2020 at 11:13:38AM -0700, Boris Burkov wrote: > #include "block-group.h" > #include "discard.h" > +#include "free-space-tree.h" > > #include "qgroup.h" > #define CREATE_TRACE_POINTS > @@ -1838,6 +1839,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) > u64 old_max_inline = fs_info->max_inline; > u32 old_thread_pool_size = fs_info->thread_pool_size; > u32 old_metadata_ratio = fs_info->metadata_ratio; > + bool create_fst = false; > int ret; > > sync_filesystem(sb); > @@ -1862,6 +1864,16 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) > btrfs_resize_thread_pool(fs_info, > fs_info->thread_pool_size, old_thread_pool_size); > > + if (btrfs_test_opt(fs_info, FREE_SPACE_TREE) && > + !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { > + create_fst = true; > + if (!sb_rdonly(sb) || *flags & SB_RDONLY) { > + btrfs_warn(fs_info, > + "Remounting with free space tree only supported from read-only to read-write"); lowercase for start of the sting, unindent it so ends below 80 columns > + create_fst = false; > + } > + } > + > if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb)) > goto out; > > @@ -1924,6 +1936,21 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) > goto restore; > } > > + /* > + * NOTE: when remounting with a change that does writes, don't > + * put it anywhere above this point, as we are not sure to be > + * safe to write until we pass the above checks. > + */ > + if (create_fst) { > + ret = btrfs_create_free_space_tree(fs_info); > + if (ret) { > + btrfs_warn(fs_info, > + "failed to create free space tree: %d", ret); same > + goto restore; > + } > + } > + > + > ret = btrfs_cleanup_fs_roots(fs_info); > if (ret) > goto restore; > -- > 2.24.1
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 3ebe7240c63d..8dfd6089e31d 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -47,6 +47,7 @@ #include "tests/btrfs-tests.h" #include "block-group.h" #include "discard.h" +#include "free-space-tree.h" #include "qgroup.h" #define CREATE_TRACE_POINTS @@ -1838,6 +1839,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) u64 old_max_inline = fs_info->max_inline; u32 old_thread_pool_size = fs_info->thread_pool_size; u32 old_metadata_ratio = fs_info->metadata_ratio; + bool create_fst = false; int ret; sync_filesystem(sb); @@ -1862,6 +1864,16 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) btrfs_resize_thread_pool(fs_info, fs_info->thread_pool_size, old_thread_pool_size); + if (btrfs_test_opt(fs_info, FREE_SPACE_TREE) && + !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) { + create_fst = true; + if (!sb_rdonly(sb) || *flags & SB_RDONLY) { + btrfs_warn(fs_info, + "Remounting with free space tree only supported from read-only to read-write"); + create_fst = false; + } + } + if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb)) goto out; @@ -1924,6 +1936,21 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) goto restore; } + /* + * NOTE: when remounting with a change that does writes, don't + * put it anywhere above this point, as we are not sure to be + * safe to write until we pass the above checks. + */ + if (create_fst) { + ret = btrfs_create_free_space_tree(fs_info); + if (ret) { + btrfs_warn(fs_info, + "failed to create free space tree: %d", ret); + goto restore; + } + } + + ret = btrfs_cleanup_fs_roots(fs_info); if (ret) goto restore;
When a user attempts to remount a btrfs filesystem with 'mount -o remount,space_cache=v2', that operation silently succeeds. Unfortunately, this is misleading, because the remount does not create the free space tree. /proc/mounts will incorrectly show space_cache=v2, but on the next mount, the file system will revert to the old space_cache. For now, we handle only the easier case, where the existing mount is read-only and the new mount is read-write. In that case, we can create the free space tree without contending with the block groups changing as we go. If the remount is ro->ro, rw->ro, or rw->rw, we will not create the free space tree, and print a warning to dmesg so that this failure is more visible. References: https://github.com/btrfs/btrfs-todo/issues/5 Signed-off-by: Boris Burkov <boris@bur.io> --- v3: - change failure to warning in dmesg for consistency v2: - move creation down to ro->rw case - error on all other remount cases - add a comment to help future remount modifiers fs/btrfs/super.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)