* [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
[parent not found: <CGME20220412112747eucas1p11e2747339407a5c51a93e7b7060ab965@eucas1p1.samsung.com>]
* [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
* 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
[parent not found: <CGME20220412112748eucas1p19a9e013fa04d5a82abd5364604a8ad31@eucas1p1.samsung.com>]
* [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
* 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 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 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
[parent not found: <CGME20220412112749eucas1p28b82e6b3b563f2e25c78f479c1192451@eucas1p2.samsung.com>]
* [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 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 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 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 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).