linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size
       [not found] <CGME20220412112746eucas1p14f52961641ef07fdc7274e75887da9ad@eucas1p1.samsung.com>
@ 2022-04-12 11:27 ` Pankaj Raghav
       [not found]   ` <CGME20220412112747eucas1p11e2747339407a5c51a93e7b7060ab965@eucas1p1.samsung.com>
                     ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-12 11:27 UTC (permalink / raw)
  To: linux-f2fs-devel
  Cc: Pankaj Raghav, Damien.LeMoal, pankydev8, mcgrof, javier.gonz

F2FS only works for zoned devices with power-of-2 zone sizes as the
f2fs section needs to be a power-of-2.

At the moment the linux kernel only accepts zoned devices which are
power-of-2 as block devices but there are zoned devices in the market
which have non power-of-2 zone sizes.

As a proactive measure, this patchset adds a check to return error
from mkfs and fsck for zoned devices with non power-of-2 zone sizes.

Luis Chamberlain (3):
  libf2fs_zoned: factor out helper to get chunk sectors
  libf2fs: add support to report zone size
  libf2fs: don't allow mkfs / fsck on zoned NPO2

 include/f2fs_fs.h   |  1 +
 lib/libf2fs.c       | 17 +++++++++++++++--
 lib/libf2fs_zoned.c | 32 ++++++++++++++++++++++----------
 3 files changed, 38 insertions(+), 12 deletions(-)

-- 
2.25.1



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [f2fs-dev] [PATCH 1/3] libf2fs_zoned: factor out helper to get chunk sectors
       [not found]   ` <CGME20220412112747eucas1p11e2747339407a5c51a93e7b7060ab965@eucas1p1.samsung.com>
@ 2022-04-12 11:27     ` Pankaj Raghav
  2022-04-12 12:17       ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 1 reply; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-12 11:27 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: javier.gonz, mcgrof, Damien.LeMoal, pankydev8

From: Luis Chamberlain <mcgrof@kernel.org>

This moves the code which fetches the chunk_sectors into a helper.
Yes, this could in theory be used by non-zoned devices but that's
not the case yet, so no need to make this a generic routine.

This makes it clear what this is doing, and helps us make the
subsequent changes easier to read.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 lib/libf2fs_zoned.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
index ce73b9a..1447181 100644
--- a/lib/libf2fs_zoned.c
+++ b/lib/libf2fs_zoned.c
@@ -146,35 +146,46 @@ int f2fs_get_zoned_model(int i)
 	return 0;
 }
 
-int f2fs_get_zone_blocks(int i)
+uint64_t f2fs_get_zone_chunk_sectors(struct device_info *dev)
 {
-	struct device_info *dev = c.devices + i;
 	uint64_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);
+	if (!sectors)
+		return 0;
+
+	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;
 
-- 
2.25.1



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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size
       [not found]   ` <CGME20220412112748eucas1p19a9e013fa04d5a82abd5364604a8ad31@eucas1p1.samsung.com>
@ 2022-04-12 11:27     ` Pankaj Raghav
  2022-04-12 12:14       ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 1 reply; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-12 11:27 UTC (permalink / raw)
  To: linux-f2fs-devel
  Cc: Pankaj Raghav, Damien.LeMoal, pankydev8, mcgrof, javier.gonz

From: Luis Chamberlain <mcgrof@kernel.org>

Expand get_device_info() to report the zone size.
This is now important give power of 2 zone sizees (PO2)
can exist, and so can non power of 2 zones sizes (NPO2),
and we should be aware of the differences in terms of
support.

This will be used more in subsequent patch.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 include/f2fs_fs.h   | 1 +
 lib/libf2fs.c       | 5 +++--
 lib/libf2fs_zoned.c | 5 +++--
 3 files changed, 7 insertions(+), 4 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..8fad1d7 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -1055,8 +1055,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: %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 1447181..0acae88 100644
--- a/lib/libf2fs_zoned.c
+++ b/lib/libf2fs_zoned.c
@@ -189,8 +189,9 @@ int f2fs_get_zone_blocks(int i)
 	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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [f2fs-dev] [PATCH 3/3] libf2fs: don't allow mkfs / fsck on zoned NPO2
       [not found]   ` <CGME20220412112749eucas1p28b82e6b3b563f2e25c78f479c1192451@eucas1p2.samsung.com>
@ 2022-04-12 11:27     ` Pankaj Raghav
  2022-04-12 12:16       ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 1 reply; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-12 11:27 UTC (permalink / raw)
  To: linux-f2fs-devel
  Cc: Pankaj Raghav, Damien.LeMoal, pankydev8, mcgrof, javier.gonz

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
zone is device is found, and if so disallow its use. This prevents
users from incorrectly using these devices.

This is a non-issue today give today's kernel does not allow NPO2
zon devices to exist. But these devices do exist, and so proactively
put a stop-gap measure in place to prevent the from being assumed
to be used.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 lib/libf2fs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 8fad1d7..a13ba32 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)
+{
+	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 (!dev->zone_size || !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.
-- 
2.25.1



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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size
  2022-04-12 11:27     ` [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size Pankaj Raghav
@ 2022-04-12 12:14       ` Damien Le Moal via Linux-f2fs-devel
  2022-04-12 16:23         ` Luis Chamberlain
  0 siblings, 1 reply; 14+ messages in thread
From: Damien Le Moal via Linux-f2fs-devel @ 2022-04-12 12:14 UTC (permalink / raw)
  To: p.raghav@samsung.com, linux-f2fs-devel@lists.sourceforge.net
  Cc: javier.gonz@samsung.com, mcgrof@kernel.org, pankydev8@gmail.com

On Tue, 2022-04-12 at 13:27 +0200, Pankaj Raghav wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> Expand get_device_info() to report the zone size.
> This is now important give power of 2 zone sizees (PO2)

s/give/given that
s/sizees/size

> can exist, and so can non power of 2 zones sizes (NPO2),

No they cannot, not yet in Linux.

> and we should be aware of the differences in terms of
> support.
> 
> This will be used more in subsequent patch.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  include/f2fs_fs.h   | 1 +
>  lib/libf2fs.c       | 5 +++--
>  lib/libf2fs_zoned.c | 5 +++--
>  3 files changed, 7 insertions(+), 4 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..8fad1d7 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -1055,8 +1055,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: %u randomly writeable zones\n",

No unit mentioned for zone size in the message.

> +				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 1447181..0acae88 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -189,8 +189,9 @@ int f2fs_get_zone_blocks(int i)
>  	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

Overall, I really do not see the point of this patch.

-- 
Damien Le Moal
Western Digital Research


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 3/3] libf2fs: don't allow mkfs / fsck on zoned NPO2
  2022-04-12 11:27     ` [f2fs-dev] [PATCH 3/3] libf2fs: don't allow mkfs / fsck on zoned NPO2 Pankaj Raghav
@ 2022-04-12 12:16       ` Damien Le Moal via Linux-f2fs-devel
  2022-04-12 15:30         ` Pankaj Raghav
  0 siblings, 1 reply; 14+ messages in thread
From: Damien Le Moal via Linux-f2fs-devel @ 2022-04-12 12:16 UTC (permalink / raw)
  To: p.raghav@samsung.com, linux-f2fs-devel@lists.sourceforge.net
  Cc: javier.gonz@samsung.com, mcgrof@kernel.org, pankydev8@gmail.com

On Tue, 2022-04-12 at 13:27 +0200, 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
> zone is device is found, and if so disallow its use. This prevents
> users from incorrectly using these devices.
> 
> This is a non-issue today give today's kernel does not allow NPO2
> zon devices to exist. But these devices do exist, and so proactively
> put a stop-gap measure in place to prevent the from being assumed
> to be used.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  lib/libf2fs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 8fad1d7..a13ba32 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)
> +{
> +	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 (!dev->zone_size || !is_power_of_2(dev->zone_size)) {
> +			MSG(0, "\tError: zoned: illegal zone size %lu (not a power of 2)\n",
> +					dev->zone_size);

The message should be different for the !dev->zone_size case since that
would be an error.

> +			free(stat_buf);
> +			return -1;
> +		}
> +
>  		/*
>  		 * Check zone configuration: for the first disk of a
>  		 * multi-device volume, conventional zones are needed.

Of the 3 patches of this series, this one is the only one that makes sense
to me. I fail to see how the first 2 patches improve things.

-- 
Damien Le Moal
Western Digital Research


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 1/3] libf2fs_zoned: factor out helper to get chunk sectors
  2022-04-12 11:27     ` [f2fs-dev] [PATCH 1/3] libf2fs_zoned: factor out helper to get chunk sectors Pankaj Raghav
@ 2022-04-12 12:17       ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 0 replies; 14+ messages in thread
From: Damien Le Moal via Linux-f2fs-devel @ 2022-04-12 12:17 UTC (permalink / raw)
  To: p.raghav@samsung.com, linux-f2fs-devel@lists.sourceforge.net
  Cc: javier.gonz@samsung.com, mcgrof@kernel.org, pankydev8@gmail.com

On Tue, 2022-04-12 at 13:27 +0200, Pankaj Raghav wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> This moves the code which fetches the chunk_sectors into a helper.
> Yes, this could in theory be used by non-zoned devices but that's
> not the case yet, so no need to make this a generic routine.
> 
> This makes it clear what this is doing, and helps us make the
> subsequent changes easier to read.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Pankaj, since you are sending the patch, add your Signed-ff-by please.

> ---
>  lib/libf2fs_zoned.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> index ce73b9a..1447181 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -146,35 +146,46 @@ int f2fs_get_zoned_model(int i)
>  	return 0;
>  }
>  
> -int f2fs_get_zone_blocks(int i)
> +uint64_t f2fs_get_zone_chunk_sectors(struct device_info *dev)

chunk_sectors is an unsigned int. Why the change to uint64_t ?

>  {
> -	struct device_info *dev = c.devices + i;
>  	uint64_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);
> +	if (!sectors)
> +		return 0;
> +
> +	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;
>  

-- 
Damien Le Moal
Western Digital Research


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 3/3] libf2fs: don't allow mkfs / fsck on zoned NPO2
  2022-04-12 12:16       ` Damien Le Moal via Linux-f2fs-devel
@ 2022-04-12 15:30         ` Pankaj Raghav
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-12 15:30 UTC (permalink / raw)
  To: Damien Le Moal, linux-f2fs-devel@lists.sourceforge.net
  Cc: javier.gonz@samsung.com, mcgrof@kernel.org, pankydev8@gmail.com

Hi Damien,

On 2022-04-12 14:16, Damien Le Moal wrote:>>  int get_device_info(int i)
>>  {
>>  	int32_t fd = 0;
>> @@ -1043,6 +1048,13 @@ int get_device_info(int i)
>>  			return -1;
>>  		}
>>  
>> +		if (!dev->zone_size || !is_power_of_2(dev->zone_size)) {
>> +			MSG(0, "\tError: zoned: illegal zone size %lu (not a power of 2)\n",
>> +					dev->zone_size);
> 
> The message should be different for the !dev->zone_size case since that
> would be an error.
I just noticed that there is a check for zero value in
f2fs_get_zone_blocks. So this could be simplified with just a power of 2
check.
> 
>> +			free(stat_buf);
>> +			return -1;
>> +		}
>> +
>>  		/*
>>  		 * Check zone configuration: for the first disk of a
>>  		 * multi-device volume, conventional zones are needed.
> 
> Of the 3 patches of this series, this one is the only one that makes sense
> to me. I fail to see how the first 2 patches improve things.
> 
Probably I can squash them together with your comments in to one commit.
Thanks.


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size
  2022-04-12 12:14       ` Damien Le Moal via Linux-f2fs-devel
@ 2022-04-12 16:23         ` Luis Chamberlain
  2022-04-13  1:03           ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2022-04-12 16:23 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: p.raghav@samsung.com, javier.gonz@samsung.com,
	pankydev8@gmail.com, linux-f2fs-devel@lists.sourceforge.net

On Tue, Apr 12, 2022 at 12:14:13PM +0000, Damien Le Moal wrote:
> On Tue, 2022-04-12 at 13:27 +0200, Pankaj Raghav wrote:
> > From: Luis Chamberlain <mcgrof@kernel.org>
> > 
> > Expand get_device_info() to report the zone size.
> > This is now important give power of 2 zone sizees (PO2)
> 
> s/give/given that
> s/sizees/size
> 
> > can exist, and so can non power of 2 zones sizes (NPO2),
> 
> No they cannot, not yet in Linux.

They *do* exist in the real world, and in Linux they do come up when
users are manually removing the current po2 requirement on Linux
sources [0].

So how about:

This is now important give power of 2 zone sizees (PO2)
do exist and some users are manually forcing to enable these
on Linux [0].

[0] https://lkml.kernel.org/r/CAJjM=8Cap1bwu8cp-mRTsiBmbHH=Ed8pp9vdAsig2o_ZiHTc-g@mail.gmail.com

> Overall, I really do not see the point of this patch.

It makes the subsequent patch easier to read and also makes
the displaying zone support separate easier to review. It can
certainly be merged, but I don't think it makes the last patch
as easier to read. It's subjective and up to the maintainer.

  Luis


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size
  2022-04-12 11:27 ` [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size Pankaj Raghav
                     ` (2 preceding siblings ...)
       [not found]   ` <CGME20220412112749eucas1p28b82e6b3b563f2e25c78f479c1192451@eucas1p2.samsung.com>
@ 2022-04-12 16:33   ` Jaegeuk Kim
  2022-04-13 11:14     ` Pankaj Raghav
  3 siblings, 1 reply; 14+ messages in thread
From: Jaegeuk Kim @ 2022-04-12 16:33 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: javier.gonz, Damien.LeMoal, mcgrof, pankydev8, linux-f2fs-devel

On 04/12, Pankaj Raghav wrote:
> F2FS only works for zoned devices with power-of-2 zone sizes as the
> f2fs section needs to be a power-of-2.

The section size actually supports multiple 2MBs, not PO2.

> 
> At the moment the linux kernel only accepts zoned devices which are
> power-of-2 as block devices but there are zoned devices in the market
> which have non power-of-2 zone sizes.
> 
> As a proactive measure, this patchset adds a check to return error
> from mkfs and fsck for zoned devices with non power-of-2 zone sizes.
> 
> Luis Chamberlain (3):
>   libf2fs_zoned: factor out helper to get chunk sectors
>   libf2fs: add support to report zone size
>   libf2fs: don't allow mkfs / fsck on zoned NPO2
> 
>  include/f2fs_fs.h   |  1 +
>  lib/libf2fs.c       | 17 +++++++++++++++--
>  lib/libf2fs_zoned.c | 32 ++++++++++++++++++++++----------
>  3 files changed, 38 insertions(+), 12 deletions(-)
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size
  2022-04-12 16:23         ` Luis Chamberlain
@ 2022-04-13  1:03           ` Damien Le Moal via Linux-f2fs-devel
  0 siblings, 0 replies; 14+ messages in thread
From: Damien Le Moal via Linux-f2fs-devel @ 2022-04-13  1:03 UTC (permalink / raw)
  To: mcgrof@kernel.org
  Cc: p.raghav@samsung.com, javier.gonz@samsung.com,
	pankydev8@gmail.com, linux-f2fs-devel@lists.sourceforge.net

On Tue, 2022-04-12 at 09:23 -0700, Luis Chamberlain wrote:
> On Tue, Apr 12, 2022 at 12:14:13PM +0000, Damien Le Moal wrote:
> > On Tue, 2022-04-12 at 13:27 +0200, Pankaj Raghav wrote:
> > > From: Luis Chamberlain <mcgrof@kernel.org>
> > > 
> > > Expand get_device_info() to report the zone size.
> > > This is now important give power of 2 zone sizees (PO2)
> > 
> > s/give/given that
> > s/sizees/size
> > 
> > > can exist, and so can non power of 2 zones sizes (NPO2),
> > 
> > No they cannot, not yet in Linux.
> 
> They *do* exist in the real world, and in Linux they do come up when
> users are manually removing the current po2 requirement on Linux
> sources [0].
> 
> So how about:
> 
> This is now important give power of 2 zone sizees (PO2)
> do exist and some users are manually forcing to enable these
> on Linux [0].

Please fix the grammar and typos as mentioned above.

> 
> [0] https://lkml.kernel.org/r/CAJjM=8Cap1bwu8cp-mRTsiBmbHH=Ed8pp9vdAsig2o_ZiHTc-g@mail.gmail.com

This link does not work: lore says "not found"

> 
> > Overall, I really do not see the point of this patch.
> 
> It makes the subsequent patch easier to read and also makes
> the displaying zone support separate easier to review. It can
> certainly be merged, but I don't think it makes the last patch
> as easier to read. It's subjective and up to the maintainer.

In my opinion, a single patch that adds power of 2 check right after
chunk_sectors is read would be a lot simpler and easier to understand.

> 
>   Luis

-- 
Damien Le Moal
Western Digital Research


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size
  2022-04-12 16:33   ` [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size Jaegeuk Kim
@ 2022-04-13 11:14     ` Pankaj Raghav
  2022-04-13 16:22       ` Jaegeuk Kim
  0 siblings, 1 reply; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-13 11:14 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: javier.gonz, Damien.LeMoal, mcgrof, pankydev8, linux-f2fs-devel

Hi Jaegeuk,

On 2022-04-12 18:33, Jaegeuk Kim wrote:
> On 04/12, Pankaj Raghav wrote:
>> F2FS only works for zoned devices with power-of-2 zone sizes as the
>> f2fs section needs to be a power-of-2.
> 
> The section size actually supports multiple 2MBs, not PO2.
> 
Thanks a lot for the clarification. I will remove this statement in the
next revision.

I was partially misled by [1] where it is stated "Segments are collected
into sections. There is genuine flexibility in the size of a section,
though it must be a power of two.".

Just FYI, when I did a quick check, there are some assumptions in the
zoned support for f2fs which assumes the zoned device size is a power of
2 such as in the __f2fs_issue_discard_zone. So if I am not wrong, when
we remove those assumptions in f2fs for zone size, then everything
should work fine provided the zone size is a multiple of 2MB. Am I
missing something here?

I am new to f2fs but is there testsuite that I can run for f2fs apart
from the two tests listed in (x)fstests?

[1] [https://lwn.net/Articles/518988/](An f2fs teardown)


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size
  2022-04-13 11:14     ` Pankaj Raghav
@ 2022-04-13 16:22       ` Jaegeuk Kim
  2022-04-13 17:53         ` Pankaj Raghav
  0 siblings, 1 reply; 14+ messages in thread
From: Jaegeuk Kim @ 2022-04-13 16:22 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: javier.gonz, Damien.LeMoal, mcgrof, pankydev8, linux-f2fs-devel

Hi Pankaj,

On 04/13, Pankaj Raghav wrote:
> Hi Jaegeuk,
> 
> On 2022-04-12 18:33, Jaegeuk Kim wrote:
> > On 04/12, Pankaj Raghav wrote:
> >> F2FS only works for zoned devices with power-of-2 zone sizes as the
> >> f2fs section needs to be a power-of-2.
> > 
> > The section size actually supports multiple 2MBs, not PO2.
> > 
> Thanks a lot for the clarification. I will remove this statement in the
> next revision.
> 
> I was partially misled by [1] where it is stated "Segments are collected
> into sections. There is genuine flexibility in the size of a section,
> though it must be a power of two.".
> 
> Just FYI, when I did a quick check, there are some assumptions in the
> zoned support for f2fs which assumes the zoned device size is a power of
> 2 such as in the __f2fs_issue_discard_zone. So if I am not wrong, when
> we remove those assumptions in f2fs for zone size, then everything
> should work fine provided the zone size is a multiple of 2MB. Am I
> missing something here?

All the implementaion assumes PO2 by block layer in kernel, but basically
f2fs could support 2MBs. So, I remember there's no PO2 check in f2fs as such.

> 
> I am new to f2fs but is there testsuite that I can run for f2fs apart
> from the two tests listed in (x)fstests?

I usually run 1) full xfstests, 2) loop of fsstress + shutdown. You can find
a script here. :)

https://github.com/jaegeuk/xfstests-f2fs/blob/f2fs/run.sh

> 
> [1] [https://lwn.net/Articles/518988/](An f2fs teardown)


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size
  2022-04-13 16:22       ` Jaegeuk Kim
@ 2022-04-13 17:53         ` Pankaj Raghav
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav @ 2022-04-13 17:53 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: javier.gonz, Damien.LeMoal, mcgrof, pankydev8, linux-f2fs-devel

Hi Jaegeuk,

On 2022-04-13 18:22, Jaegeuk Kim wrote:
>>> The section size actually supports multiple 2MBs, not PO2.
>>>
>> Thanks a lot for the clarification. I will remove this statement in the
>> next revision.
>>
>> I was partially misled by [1] where it is stated "Segments are collected
>> into sections. There is genuine flexibility in the size of a section,
>> though it must be a power of two.".
>>
>> Just FYI, when I did a quick check, there are some assumptions in the
>> zoned support for f2fs which assumes the zoned device size is a power of
>> 2 such as in the __f2fs_issue_discard_zone. So if I am not wrong, when
>> we remove those assumptions in f2fs for zone size, then everything
>> should work fine provided the zone size is a multiple of 2MB. Am I
>> missing something here?
> 
> All the implementaion assumes PO2 by block layer in kernel, but basically
Yeah, at the moment it is not an issue as kernel rejects non power of 2
zoned devices as a block device. But we have been having some
conversation around this topic [1] to remove this constraint from the
block layer.

> f2fs could support 2MBs. So, I remember there's no PO2 check in f2fs as such.
> 
>>
>> I am new to f2fs but is there testsuite that I can run for f2fs apart
>> from the two tests listed in (x)fstests?
> 
> I usually run 1) full xfstests, 2) loop of fsstress + shutdown. You can find
> a script here. :)
> 
> https://github.com/jaegeuk/xfstests-f2fs/blob/f2fs/run.sh
>
Awesome, thanks a lot :)

[1] https://lore.kernel.org/all/20220315135245.eqf4tqngxxb7ymqa@unifi/


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-04-13 17:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20220412112746eucas1p14f52961641ef07fdc7274e75887da9ad@eucas1p1.samsung.com>
2022-04-12 11:27 ` [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size Pankaj Raghav
     [not found]   ` <CGME20220412112747eucas1p11e2747339407a5c51a93e7b7060ab965@eucas1p1.samsung.com>
2022-04-12 11:27     ` [f2fs-dev] [PATCH 1/3] libf2fs_zoned: factor out helper to get chunk sectors Pankaj Raghav
2022-04-12 12:17       ` Damien Le Moal via Linux-f2fs-devel
     [not found]   ` <CGME20220412112748eucas1p19a9e013fa04d5a82abd5364604a8ad31@eucas1p1.samsung.com>
2022-04-12 11:27     ` [f2fs-dev] [PATCH 2/3] libf2fs: add support to report zone size Pankaj Raghav
2022-04-12 12:14       ` Damien Le Moal via Linux-f2fs-devel
2022-04-12 16:23         ` Luis Chamberlain
2022-04-13  1:03           ` Damien Le Moal via Linux-f2fs-devel
     [not found]   ` <CGME20220412112749eucas1p28b82e6b3b563f2e25c78f479c1192451@eucas1p2.samsung.com>
2022-04-12 11:27     ` [f2fs-dev] [PATCH 3/3] libf2fs: don't allow mkfs / fsck on zoned NPO2 Pankaj Raghav
2022-04-12 12:16       ` Damien Le Moal via Linux-f2fs-devel
2022-04-12 15:30         ` Pankaj Raghav
2022-04-12 16:33   ` [f2fs-dev] [PATCH 0/3] f2fs-tools: return error for zoned devices with non power-of-2 zone size Jaegeuk Kim
2022-04-13 11:14     ` Pankaj Raghav
2022-04-13 16:22       ` Jaegeuk Kim
2022-04-13 17:53         ` Pankaj Raghav

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).