Re: [f2fs-dev] [PATCH v2] libf2fs: don't allow mkfs / fsck on non power-of-2 zoned devices

[Jaegeuk Kim <jaegeuk@kernel.org>]

On 04/13, Pankaj Raghav wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> f2fs currently only work with zoned storage devices with a zone
> size which is a power of 2 (PO2). So check if a non-power of 2
> zoned device is found, and if so disallow its use. This prevents
> users from incorrectly using these devices.
> 
> This is a non-issue today given today's kernel does not allow NPO2
> zoned devices to exist as a block device. But NPO2 zoned devices do exist
> so proactively put a stop-gap measure in place to prevent it from being
> incorrectly used.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> Changes since v1:
> - Squash the commits for clarity (Damien)
> - f2fs_get_zone_chunk_sectors can return uint32_t (Damien)
> - Include the units for zone size in msg info (Damien)
> - Sections can be npo2 but it should only be a multiple of 2MB (Jaegeuk)
> 
>  include/f2fs_fs.h   |  1 +
>  lib/libf2fs.c       | 17 +++++++++++++++--
>  lib/libf2fs_zoned.c | 34 ++++++++++++++++++++++------------
>  3 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index d236437..83c5b33 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -386,6 +386,7 @@ struct device_info {
>  	u_int32_t nr_zones;
>  	u_int32_t nr_rnd_zones;
>  	size_t zone_blocks;
> +	uint64_t zone_size;
>  	size_t *zone_cap_blocks;
>  };
>  
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 420dfda..8104667 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -882,6 +882,11 @@ static int open_check_fs(char *path, int flag)
>  	return open(path, O_RDONLY | flag);
>  }
>  
> +static int is_power_of_2(unsigned long n)

So, this needs to check 2MB alignment only?

> +{
> +	return (n != 0 && ((n & (n - 1)) == 0));
> +}
> +
>  int get_device_info(int i)
>  {
>  	int32_t fd = 0;
> @@ -1043,6 +1048,13 @@ int get_device_info(int i)
>  			return -1;
>  		}
>  
> +		if (!is_power_of_2(dev->zone_size)) {
> +			MSG(0, "\tError: zoned: illegal zone size %lu (not a power of 2)\n",
> +					dev->zone_size);
> +			free(stat_buf);
> +			return -1;
> +		}
> +
>  		/*
>  		 * Check zone configuration: for the first disk of a
>  		 * multi-device volume, conventional zones are needed.
> @@ -1055,8 +1067,9 @@ int get_device_info(int i)
>  		MSG(0, "Info: Host-%s zoned block device:\n",
>  				(dev->zoned_model == F2FS_ZONED_HA) ?
>  					"aware" : "managed");
> -		MSG(0, "      %u zones, %u randomly writeable zones\n",
> -				dev->nr_zones, dev->nr_rnd_zones);
> +		MSG(0, "      %u zones, %lu zone size(bytes), %u randomly writeable zones\n",
> +				dev->nr_zones, dev->zone_size,
> +				dev->nr_rnd_zones);
>  		MSG(0, "      %lu blocks per zone\n",
>  				dev->zone_blocks);
>  	}
> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> index ce73b9a..48a23c0 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -146,40 +146,50 @@ int f2fs_get_zoned_model(int i)
>  	return 0;
>  }
>  
> -int f2fs_get_zone_blocks(int i)
> +uint32_t f2fs_get_zone_chunk_sectors(struct device_info *dev)
>  {
> -	struct device_info *dev = c.devices + i;
> -	uint64_t sectors;
> +	uint32_t sectors;
>  	char str[PATH_MAX];
>  	FILE *file;
>  	int res;
>  
> -	/* Get zone size */
> -	dev->zone_blocks = 0;
> -
>  	res = get_sysfs_path(dev, "queue/chunk_sectors", str, sizeof(str));
>  	if (res != 0) {
>  		MSG(0, "\tError: Failed to get device sysfs attribute path\n");
> -		return -1;
> +		return 0;
>  	}
>  
>  	file = fopen(str, "r");
>  	if (!file)
> -		return -1;
> +		return 0;
>  
>  	memset(str, 0, sizeof(str));
>  	res = fscanf(file, "%s", str);
>  	fclose(file);
>  
>  	if (res != 1)
> -		return -1;
> +		return 0;
>  
> -	sectors = atol(str);
> +	sectors = atoi(str);
> +
> +	return sectors;
> +}
> +
> +int f2fs_get_zone_blocks(int i)
> +{
> +	struct device_info *dev = c.devices + i;
> +	uint64_t sectors;
> +
> +	/* Get zone size */
> +	dev->zone_blocks = 0;
> +
> +	sectors = f2fs_get_zone_chunk_sectors(dev);
>  	if (!sectors)
>  		return -1;
>  
> -	dev->zone_blocks = sectors >> (F2FS_BLKSIZE_BITS - 9);
> -	sectors = (sectors << 9) / c.sector_size;
> +	dev->zone_size = sectors << SECTOR_SHIFT;
> +	dev->zone_blocks = sectors >> (F2FS_BLKSIZE_BITS - SECTOR_SHIFT);
> +	sectors = dev->zone_size / c.sector_size;
>  
>  	/*
>  	 * Total number of zones: there may
> -- 
> 2.25.1
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel