* [PATCH] btrfs: clear free space cache when nospace_cache mount option is specified
@ 2021-11-02 8:42 Qu Wenruo
2021-11-02 13:08 ` Josef Bacik
0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2021-11-02 8:42 UTC (permalink / raw)
To: linux-btrfs; +Cc: Wang Yugui
[BUG]
With latest btrfs-progs v5.15.x testing branch, fstests/btrfs/215 will
fail like the following:
MKFS_OPTIONS -- /dev/mapper/test-scratch1
MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
btrfs/215 0s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/215.out.bad)
--- tests/btrfs/215.out 2020-11-03 15:05:07.920000001 +0800
+++ ~/xfstests-dev/results//btrfs/215.out.bad 2021-11-02 16:31:17.626666667 +0800
@@ -1,2 +1,4 @@
QA output created by 215
-Silence is golden
+mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/test-scratch1, missing codepage or helper program, or other error.
+mount -o nospace_cache /dev/mapper/test-scratch1 /mnt/scratch failed
+(see ~/xfstests-dev/results//btrfs/215.full for details)
...
(Run 'diff -u ~/xfstests-dev/tests/btrfs/215.out ~/xfstests-dev/results//btrfs/215.out.bad' to see the entire diff)
Ran: btrfs/215
[CAUSE]
Currently btrfs doesn't allow mounting with nospace_cache when there is
already a v2 cache.
The logic looks like this, in btrfs_parse_options():
case Opt_no_space_cache:
if (btrfs_test_opt(info, FREE_SPACE_TREE)) {
btrfs_set_opt(info->mount_opt, CLEAR_CACHE);
btrfs_clear_and_info(info, FREE_SPACE_TREE,
"disabling and clearing free space tree");
}
break;
Then at the end of the same function:
if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
!btrfs_test_opt(info, FREE_SPACE_TREE) &&
!btrfs_test_opt(info, CLEAR_CACHE)) {
btrfs_err(info, "cannot disable free space tree");
ret = -EINVAL;
}
Thus causing above mount failure.
[FIX]
I'm not sure why we don't allow nospace_cache with v2 cache, my
assumption is we don't have generation check for v2 cache, thus if we
make v2 space cache get out of sync with the on-disk data, it can
corrupt the fs.
That needs to be properly addressed, but for now, to allow nospace_cache
with v2 cache, we can simply force to clear v2 cache when nospace_cache
mount option is specified.
Since we're here, also remove a unnecessary new line the v2 cache check
at the end btrfs_parse_options().
Reported-by: Wang Yugui <wangyugui@e16-tech.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/super.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 537d90bf5d84..59e4a756cea6 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -887,8 +887,14 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
"disabling disk space caching");
}
if (btrfs_test_opt(info, FREE_SPACE_TREE)) {
+ /*
+ * For v2 cache with nospace_cache mount option,
+ * v2 cache can get out of sync if not cleared.
+ * Thus here we set to clear v2 cache.
+ */
+ btrfs_set_opt(info->mount_opt, CLEAR_CACHE);
btrfs_clear_and_info(info, FREE_SPACE_TREE,
- "disabling free space tree");
+ "disabling and clearing free space tree");
}
break;
case Opt_inode_cache:
@@ -1036,7 +1042,6 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
!btrfs_test_opt(info, CLEAR_CACHE)) {
btrfs_err(info, "cannot disable free space tree");
ret = -EINVAL;
-
}
if (!ret)
ret = btrfs_check_mountopts_zoned(info);
--
2.33.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: clear free space cache when nospace_cache mount option is specified
2021-11-02 8:42 [PATCH] btrfs: clear free space cache when nospace_cache mount option is specified Qu Wenruo
@ 2021-11-02 13:08 ` Josef Bacik
2021-11-02 23:20 ` Qu Wenruo
0 siblings, 1 reply; 3+ messages in thread
From: Josef Bacik @ 2021-11-02 13:08 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Wang Yugui
On Tue, Nov 02, 2021 at 04:42:42PM +0800, Qu Wenruo wrote:
> [BUG]
> With latest btrfs-progs v5.15.x testing branch, fstests/btrfs/215 will
> fail like the following:
>
> MKFS_OPTIONS -- /dev/mapper/test-scratch1
> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
>
> btrfs/215 0s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/215.out.bad)
> --- tests/btrfs/215.out 2020-11-03 15:05:07.920000001 +0800
> +++ ~/xfstests-dev/results//btrfs/215.out.bad 2021-11-02 16:31:17.626666667 +0800
> @@ -1,2 +1,4 @@
> QA output created by 215
> -Silence is golden
> +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/test-scratch1, missing codepage or helper program, or other error.
> +mount -o nospace_cache /dev/mapper/test-scratch1 /mnt/scratch failed
> +(see ~/xfstests-dev/results//btrfs/215.full for details)
> ...
> (Run 'diff -u ~/xfstests-dev/tests/btrfs/215.out ~/xfstests-dev/results//btrfs/215.out.bad' to see the entire diff)
> Ran: btrfs/215
>
> [CAUSE]
> Currently btrfs doesn't allow mounting with nospace_cache when there is
> already a v2 cache.
>
> The logic looks like this, in btrfs_parse_options():
>
> case Opt_no_space_cache:
> if (btrfs_test_opt(info, FREE_SPACE_TREE)) {
> btrfs_set_opt(info->mount_opt, CLEAR_CACHE);
> btrfs_clear_and_info(info, FREE_SPACE_TREE,
> "disabling and clearing free space tree");
> }
> break;
>
> Then at the end of the same function:
>
> if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
> !btrfs_test_opt(info, FREE_SPACE_TREE) &&
> !btrfs_test_opt(info, CLEAR_CACHE)) {
> btrfs_err(info, "cannot disable free space tree");
> ret = -EINVAL;
> }
>
> Thus causing above mount failure.
>
> [FIX]
> I'm not sure why we don't allow nospace_cache with v2 cache, my
> assumption is we don't have generation check for v2 cache, thus if we
> make v2 space cache get out of sync with the on-disk data, it can
> corrupt the fs.
>
> That needs to be properly addressed, but for now, to allow nospace_cache
> with v2 cache, we can simply force to clear v2 cache when nospace_cache
> mount option is specified.
>
> Since we're here, also remove a unnecessary new line the v2 cache check
> at the end btrfs_parse_options().
>
> Reported-by: Wang Yugui <wangyugui@e16-tech.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
I'd rather we just not allow nospace_cache with the free space tree. It was an
option meant for the original space cache, not for the free space tree. I don't
want to surprise clear the free space tree for an unrelated mount option.
Thanks,
Josef
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: clear free space cache when nospace_cache mount option is specified
2021-11-02 13:08 ` Josef Bacik
@ 2021-11-02 23:20 ` Qu Wenruo
0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2021-11-02 23:20 UTC (permalink / raw)
To: Josef Bacik, Qu Wenruo; +Cc: linux-btrfs, Wang Yugui
On 2021/11/2 21:08, Josef Bacik wrote:
> On Tue, Nov 02, 2021 at 04:42:42PM +0800, Qu Wenruo wrote:
>> [BUG]
>> With latest btrfs-progs v5.15.x testing branch, fstests/btrfs/215 will
>> fail like the following:
>>
>> MKFS_OPTIONS -- /dev/mapper/test-scratch1
>> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
>>
>> btrfs/215 0s ... [failed, exit status 1]- output mismatch (see ~/xfstests-dev/results//btrfs/215.out.bad)
>> --- tests/btrfs/215.out 2020-11-03 15:05:07.920000001 +0800
>> +++ ~/xfstests-dev/results//btrfs/215.out.bad 2021-11-02 16:31:17.626666667 +0800
>> @@ -1,2 +1,4 @@
>> QA output created by 215
>> -Silence is golden
>> +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/test-scratch1, missing codepage or helper program, or other error.
>> +mount -o nospace_cache /dev/mapper/test-scratch1 /mnt/scratch failed
>> +(see ~/xfstests-dev/results//btrfs/215.full for details)
>> ...
>> (Run 'diff -u ~/xfstests-dev/tests/btrfs/215.out ~/xfstests-dev/results//btrfs/215.out.bad' to see the entire diff)
>> Ran: btrfs/215
>>
>> [CAUSE]
>> Currently btrfs doesn't allow mounting with nospace_cache when there is
>> already a v2 cache.
>>
>> The logic looks like this, in btrfs_parse_options():
>>
>> case Opt_no_space_cache:
>> if (btrfs_test_opt(info, FREE_SPACE_TREE)) {
>> btrfs_set_opt(info->mount_opt, CLEAR_CACHE);
>> btrfs_clear_and_info(info, FREE_SPACE_TREE,
>> "disabling and clearing free space tree");
>> }
>> break;
>>
>> Then at the end of the same function:
>>
>> if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
>> !btrfs_test_opt(info, FREE_SPACE_TREE) &&
>> !btrfs_test_opt(info, CLEAR_CACHE)) {
>> btrfs_err(info, "cannot disable free space tree");
>> ret = -EINVAL;
>> }
>>
>> Thus causing above mount failure.
>>
>> [FIX]
>> I'm not sure why we don't allow nospace_cache with v2 cache, my
>> assumption is we don't have generation check for v2 cache, thus if we
>> make v2 space cache get out of sync with the on-disk data, it can
>> corrupt the fs.
>>
>> That needs to be properly addressed, but for now, to allow nospace_cache
>> with v2 cache, we can simply force to clear v2 cache when nospace_cache
>> mount option is specified.
>>
>> Since we're here, also remove a unnecessary new line the v2 cache check
>> at the end btrfs_parse_options().
>>
>> Reported-by: Wang Yugui <wangyugui@e16-tech.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> I'd rather we just not allow nospace_cache with the free space tree. It was an
> option meant for the original space cache, not for the free space tree. I don't
> want to surprise clear the free space tree for an unrelated mount option.
OK, then we need to update the test cases to use v2 cache.
While I'm not sure fstests guys won't complain as they may want to run
it on much older kernels.
Thanks,
Qu
> Thanks,
>
> Josef
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-02 23:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-02 8:42 [PATCH] btrfs: clear free space cache when nospace_cache mount option is specified Qu Wenruo
2021-11-02 13:08 ` Josef Bacik
2021-11-02 23:20 ` Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox