linux-kernel - Re: [PATCH 03/19] btrfs: Check and enable HMZONED mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jun 2019 06:43:52 +0000
From:   Naohiro Aota <Naohiro.Aota@....com>
To:     Josef Bacik <josef@...icpanda.com>
CC:     "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
        David Sterba <dsterba@...e.com>, Chris Mason <clm@...com>,
        Qu Wenruo <wqu@...e.com>, Nikolay Borisov <nborisov@...e.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Hannes Reinecke <hare@...e.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        Damien Le Moal <Damien.LeMoal@....com>,
        Matias Bjørling <mb@...htnvm.io>,
        Johannes Thumshirn <jthumshirn@...e.de>,
        Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH 03/19] btrfs: Check and enable HMZONED mode

On 2019/06/13 22:57, Josef Bacik wrote:
> On Fri, Jun 07, 2019 at 10:10:09PM +0900, Naohiro Aota wrote:
>> HMZONED mode cannot be used together with the RAID5/6 profile for now.
>> Introduce the function btrfs_check_hmzoned_mode() to check this. This
>> function will also check if HMZONED flag is enabled on the file system and
>> if the file system consists of zoned devices with equal zone size.
>>
>> Additionally, as updates to the space cache are in-place, the space cache
>> cannot be located over sequential zones and there is no guarantees that the
>> device will have enough conventional zones to store this cache. Resolve
>> this problem by disabling completely the space cache.  This does not
>> introduces any problems with sequential block groups: all the free space is
>> located after the allocation pointer and no free space before the pointer.
>> There is no need to have such cache.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@....com>
>> Signed-off-by: Naohiro Aota <naohiro.aota@....com>
>> ---
>>   fs/btrfs/ctree.h       |  3 ++
>>   fs/btrfs/dev-replace.c |  7 +++
>>   fs/btrfs/disk-io.c     |  7 +++
>>   fs/btrfs/super.c       | 12 ++---
>>   fs/btrfs/volumes.c     | 99 ++++++++++++++++++++++++++++++++++++++++++
>>   fs/btrfs/volumes.h     |  1 +
>>   6 files changed, 124 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index b81c331b28fa..6c00101407e4 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -806,6 +806,9 @@ struct btrfs_fs_info {
>>   	struct btrfs_root *uuid_root;
>>   	struct btrfs_root *free_space_root;
>>   
>> +	/* Zone size when in HMZONED mode */
>> +	u64 zone_size;
>> +
>>   	/* the log root tree is a directory of all the other log roots */
>>   	struct btrfs_root *log_root_tree;
>>   
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index ee0989c7e3a9..fbe5ea2a04ed 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -201,6 +201,13 @@ static int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>>   		return PTR_ERR(bdev);
>>   	}
>>   
>> +	if ((bdev_zoned_model(bdev) == BLK_ZONED_HM &&
>> +	     !btrfs_fs_incompat(fs_info, HMZONED)) ||
>> +	    (!bdev_is_zoned(bdev) && btrfs_fs_incompat(fs_info, HMZONED))) {
> 
> You do this in a few places, turn this into a helper please.
> 
>> +		ret = -EINVAL;
>> +		goto error;
>> +	}
>> +
>>   	filemap_write_and_wait(bdev->bd_inode->i_mapping);
>>   
>>   	devices = &fs_info->fs_devices->devices;
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 663efce22d98..7c1404c76768 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3086,6 +3086,13 @@ int open_ctree(struct super_block *sb,
>>   
>>   	btrfs_free_extra_devids(fs_devices, 1);
>>   
>> +	ret = btrfs_check_hmzoned_mode(fs_info);
>> +	if (ret) {
>> +		btrfs_err(fs_info, "failed to init hmzoned mode: %d",
>> +				ret);
>> +		goto fail_block_groups;
>> +	}
>> +
>>   	ret = btrfs_sysfs_add_fsid(fs_devices, NULL);
>>   	if (ret) {
>>   		btrfs_err(fs_info, "failed to init sysfs fsid interface: %d",
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 2c66d9ea6a3b..740a701f16c5 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -435,11 +435,13 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>>   	bool saved_compress_force;
>>   	int no_compress = 0;
>>   
>> -	cache_gen = btrfs_super_cache_generation(info->super_copy);
>> -	if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE))
>> -		btrfs_set_opt(info->mount_opt, FREE_SPACE_TREE);
>> -	else if (cache_gen)
>> -		btrfs_set_opt(info->mount_opt, SPACE_CACHE);
>> +	if (!btrfs_fs_incompat(info, HMZONED)) {
>> +		cache_gen = btrfs_super_cache_generation(info->super_copy);
>> +		if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE))
>> +			btrfs_set_opt(info->mount_opt, FREE_SPACE_TREE);
>> +		else if (cache_gen)
>> +			btrfs_set_opt(info->mount_opt, SPACE_CACHE);
>> +	}
>>   
> 
> This disables the free space tree as well as the cache, sounds like you only
> need to disable the free space cache?  Thanks,

Right. We can still use the free space tree on HMZONED. I'll fix in the next version.
Thanks