On 23.02.21 г. 20:22 ч., Boris Burkov wrote:
> The intended logic of the check is to catch cases where the desired
> free_space_tree setting doesn't match the mounted setting, and the
> remount is anything but ro->rw. However, it makes the mistake of
> checking equality on a masked integer (btrfs_test_opt) against a boolean
> (btrfs_fs_compat_ro).
>
> If you run the reproducer:
> mount -o space_cache=v2 dev mnt
> mount -o remount,ro mnt
>
> you would expect no warning, because the remount is not attempting to
> change the free space tree setting, but we do see the warning.
>
> To fix this, convert the option test to a boolean.
>
> I tested a variety of transitions:
> sudo mount -o space_cache=v2 /dev/vg0/lv0 mnt/lol
> (fst enabled)
> mount -o remount,ro mnt/lol
> (no warning, no fst change)
> sudo mount -o remount,rw,space_cache=v1,clear_cache
> (no warning, ro->rw)
> sudo mount -o remount,rw,space_cache=v2 mnt
> (warning, rw->rw with change)
> sudo mount -o remount,ro mnt
> (no warning, no fst change)
> sudo mount -o remount,rw,space_cache=v2 mnt
> (no warning, no fst change)
>
> Reported-by: Chris Murphy <[email protected]>
> Signed-off-by: Boris Burkov <[email protected]>
> ---
> fs/btrfs/super.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index f8435641b912..d4992ceab5ea 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1918,7 +1918,7 @@ 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) !=
> + if (!!btrfs_test_opt(fs_info, FREE_SPACE_TREE) !=
I'd rather thave the !! convert to bool magic in the macro definition i.e :
#define btrfs_test_opt(fs_info, opt) !!((fs_info)->mount_opt & \
BTRFS_MOUNT_##opt)
> btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> (!sb_rdonly(sb) || (*flags & SB_RDONLY))) {
> btrfs_warn(fs_info,
>
