All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/13] support non power of 2 zoned devices
       [not found] <CGME20220516133922eucas1p1c891cd1d82539b4e792acb5d1aa74444@eucas1p1.samsung.com>
@ 2022-05-16 13:39   ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Pankaj Raghav

- Background and Motivation:

The zone storage implementation in Linux, introduced since v4.10, first
targetted SMR drives which have a power of 2 (po2) zone size alignment
requirement. The po2 zone size was further imposed implicitly by the
block layer's blk_queue_chunk_sectors(), used to prevent IO merging
across chunks beyond the specified size, since v3.16 through commit
762380ad9322 ("block: add notion of a chunk size for request merging").
But this same general block layer po2 requirement for blk_queue_chunk_sectors()
was removed on v5.10 through commit 07d098e6bbad ("block: allow 'chunk_sectors'
to be non-power-of-2").

NAND, which is the media used in newer zoned storage devices, does not
naturally align to po2. In these devices, zone cap is not the same as the
po2 zone size. When the zone cap != zone size, then unmapped LBAs are
introduced to cover the space between the zone cap and zone size. po2
requirement does not make sense for these type of zone storage devices.
This patch series aims to remove these unmapped LBAs for zoned devices when
zone cap is npo2. This is done by relaxing the po2 zone size constraint
in the kernel and allowing zoned device with npo2 zone sizes if zone cap
== zone size.

Removing the po2 requirement from zone storage should be possible
now provided that no userspace regression and no performance regressions are
introduced. Stop-gap patches have been already merged into f2fs-tools to
proactively not allow npo2 zone sizes until proper support is added [0].
Additional kernel stop-gap patches are provided in this series for dm-zoned.
Support for npo2 zonefs and btrfs support is addressed in this series.

There was an effort previously [1] to add support to non po2 devices via
device level emulation but that was rejected with a final conclusion
to add support for non po2 zoned device in the complete stack[2].

- Patchset description:
This patchset aims at adding support to non power of 2 zoned devices in
the block layer, nvme layer, null blk and adds support to btrfs and
zonefs.

This round of patches **will not** support DM layer for non
power of 2 zoned devices. More about this in the future work section.

Patches 1-2 deals with removing the po2 constraint from the
block layer.

Patches 3-4 deals with removing the constraint from nvme zns.

Patches 5-9 adds support to btrfs for non po2 zoned devices.

Patch 10 removes the po2 constraint in ZoneFS

Patch 11-12 removes the po2 contraint in null blk

Patches 13 adds conditions to not allow non power of 2 devices in
DM.

The patch series is based on linux-next tag: next-20220502

- Performance:
PO2 zone sizes utilizes log and shifts instead of division when
determing alignment, zone number, etc. The same math cannot be used when
using a zoned device with non po2 zone size. Hence, to avoid any performance
regression on zoned devices with po2 zone sizes, the optimized math in the
hot paths has been retained with branching.

The performance was measured using null blk for regression
and the results have been posted in the appropriate commit log. No
performance regression was noticed.

- Testing
With respect to testing we need to tackle two things: one for regression
on po2 zoned device and progression on non po2 zoned devices.

kdevops (https://github.com/mcgrof/kdevops) was extensively used to
automate the testing for blktests and (x)fstests for btrfs changes. The
known failures were excluded during the test based on the baseline
v5.17.0-rc7

-- regression
Emulated zoned device with zone size =128M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Changes were tested with the following profile in QEMU:
[btrfs_simple_zns]
TEST_DIR=<dir>
SCRATCH_MNT=<mnt>
FSTYP=btrfs
MKFS_OPTIONS="-f -d single -m single"
TEST_DEV=<dev>
SCRATCH_DEV_POOL=<dev-pool>

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests-nullblk.sh and zonefs-tests.sh from zonefs-tools were run
with no failures.

nullblk:
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified if dm-zoned successfully mounts without any
error.

-- progression
Emulated zoned device with zone size = 96M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Same profile as po2 zone size was used.

Many tests in xfstests for btrfs included dm-flakey and some tests
required dm-linear. As they are not supported at the moment for non
po2 devices, those **tests were excluded for non po2 devices**.

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests.sh from zonefs-tools were run with no failures.

nullblk:
A new section was added to cover non po2 devices:

section14()
{
       conv_pcnt=10
       zone_size=3
       zone_capacity=3
       max_open=${set_max_open}
       zbd_test_opts+=("-o ${max_open}")
}
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified that dm-zoned does not mount.

- Tools:
Some tools had to be updated to support non po2 devices. Once these
patches are accepted in the kernel, these tool updates will also be
upstreamed.
* btrfs-prog: https://github.com/Panky-codes/btrfs-progs/tree/remove-po2-btrfs
* blkzone: https://github.com/Panky-codes/util-linux/tree/remove-po2
* zonefs-tools: https://github.com/Panky-codes/zonefs-tools/tree/remove-po2

- Future work
To reduce the amount of changes and testing, support for DM was
excluded in this round of patches. The plan is to add support to F2FS
and DM in the forthcoming future.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test&id=6afcf6493578e77528abe65ab8b12f3e1c16749f
[1] https://lore.kernel.org/all/20220310094725.GA28499@lst.de/T/
[2] https://lore.kernel.org/all/20220315135245.eqf4tqngxxb7ymqa@unifi/

Changes since v1:
- Put the function declaration and its usage in the same commit (Bart)
- Remove bdev_zone_aligned function (Bart)
- Change the name from blk_queue_zone_aligned to blk_queue_is_zone_start
  (Damien)
- q is never null in from bdev_get_queue (Damien)
- Add condition during bringup and check for zsze == zcap for npo2
  drives (Damien)
- Rounddown operation should be made generic to work in 32 bits arch
  (bart)
- Add comments where generic calculation is directly used instead having
  special handling for po2 zone sizes (Hannes)
- Make the minimum zone size alignment requirement for btrfs to be 1M
  instead of BTRFS_STRIPE_LEN(David)

Changes since v2:
- Minor formatting changes

Changes since v3:
- Make superblock mirror align with the existing superblock log offsets
  (David)
- DM change return value and remove extra newline (Damien and Mike)
- Optimize null blk zone index lookup with shift for po2 zone size
  (Damien)

Luis Chamberlain (1):
  dm-zoned: ensure only power of 2 zone sizes are allowed

Pankaj Raghav (12):
  block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2
    zsze
  block: allow blk-zoned devices to have non-power-of-2 zone size
  nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
  nvmet: Allow ZNS target to support non-power_of_2 zone sizes
  btrfs: zoned: Cache superblock location in btrfs_zoned_device_info
  btrfs: zoned: Make sb_zone_number function non power of 2 compatible
  btrfs: zoned: use generic btrfs zone helpers to support npo2 zoned
    devices
  btrfs:zoned: make sb for npo2 zone devices align with sb log offsets
  btrfs: zoned: relax the alignment constraint for zoned devices
  zonefs: allow non power of 2 zoned devices
  null_blk: allow non power of 2 zoned devices
  null_blk: use zone_size_sects_shift for power of 2 zoned devices

 block/blk-core.c                  |   3 +-
 block/blk-zoned.c                 |  40 +++++--
 drivers/block/null_blk/main.c     |   5 +-
 drivers/block/null_blk/null_blk.h |   6 +
 drivers/block/null_blk/zoned.c    |  20 ++--
 drivers/md/dm-zone.c              |  12 ++
 drivers/nvme/host/zns.c           |  24 ++--
 drivers/nvme/target/zns.c         |   2 +-
 fs/btrfs/volumes.c                |  24 ++--
 fs/btrfs/zoned.c                  | 191 +++++++++++++++++++++---------
 fs/btrfs/zoned.h                  |  44 ++++++-
 fs/zonefs/super.c                 |   6 +-
 fs/zonefs/zonefs.h                |   1 -
 include/linux/blkdev.h            |  37 +++++-
 14 files changed, 303 insertions(+), 112 deletions(-)

-- 
2.25.1


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

* [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-16 13:39   ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Pankaj Raghav,
	linux-kernel, linux-nvme, Josef Bacik, linux-block,
	linux-fsdevel, dm-devel, Alasdair Kergon, jonathan.derrick,
	Keith Busch, Johannes Thumshirn, linux-btrfs, Sagi Grimberg

- Background and Motivation:

The zone storage implementation in Linux, introduced since v4.10, first
targetted SMR drives which have a power of 2 (po2) zone size alignment
requirement. The po2 zone size was further imposed implicitly by the
block layer's blk_queue_chunk_sectors(), used to prevent IO merging
across chunks beyond the specified size, since v3.16 through commit
762380ad9322 ("block: add notion of a chunk size for request merging").
But this same general block layer po2 requirement for blk_queue_chunk_sectors()
was removed on v5.10 through commit 07d098e6bbad ("block: allow 'chunk_sectors'
to be non-power-of-2").

NAND, which is the media used in newer zoned storage devices, does not
naturally align to po2. In these devices, zone cap is not the same as the
po2 zone size. When the zone cap != zone size, then unmapped LBAs are
introduced to cover the space between the zone cap and zone size. po2
requirement does not make sense for these type of zone storage devices.
This patch series aims to remove these unmapped LBAs for zoned devices when
zone cap is npo2. This is done by relaxing the po2 zone size constraint
in the kernel and allowing zoned device with npo2 zone sizes if zone cap
== zone size.

Removing the po2 requirement from zone storage should be possible
now provided that no userspace regression and no performance regressions are
introduced. Stop-gap patches have been already merged into f2fs-tools to
proactively not allow npo2 zone sizes until proper support is added [0].
Additional kernel stop-gap patches are provided in this series for dm-zoned.
Support for npo2 zonefs and btrfs support is addressed in this series.

There was an effort previously [1] to add support to non po2 devices via
device level emulation but that was rejected with a final conclusion
to add support for non po2 zoned device in the complete stack[2].

- Patchset description:
This patchset aims at adding support to non power of 2 zoned devices in
the block layer, nvme layer, null blk and adds support to btrfs and
zonefs.

This round of patches **will not** support DM layer for non
power of 2 zoned devices. More about this in the future work section.

Patches 1-2 deals with removing the po2 constraint from the
block layer.

Patches 3-4 deals with removing the constraint from nvme zns.

Patches 5-9 adds support to btrfs for non po2 zoned devices.

Patch 10 removes the po2 constraint in ZoneFS

Patch 11-12 removes the po2 contraint in null blk

Patches 13 adds conditions to not allow non power of 2 devices in
DM.

The patch series is based on linux-next tag: next-20220502

- Performance:
PO2 zone sizes utilizes log and shifts instead of division when
determing alignment, zone number, etc. The same math cannot be used when
using a zoned device with non po2 zone size. Hence, to avoid any performance
regression on zoned devices with po2 zone sizes, the optimized math in the
hot paths has been retained with branching.

The performance was measured using null blk for regression
and the results have been posted in the appropriate commit log. No
performance regression was noticed.

- Testing
With respect to testing we need to tackle two things: one for regression
on po2 zoned device and progression on non po2 zoned devices.

kdevops (https://github.com/mcgrof/kdevops) was extensively used to
automate the testing for blktests and (x)fstests for btrfs changes. The
known failures were excluded during the test based on the baseline
v5.17.0-rc7

-- regression
Emulated zoned device with zone size =128M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Changes were tested with the following profile in QEMU:
[btrfs_simple_zns]
TEST_DIR=<dir>
SCRATCH_MNT=<mnt>
FSTYP=btrfs
MKFS_OPTIONS="-f -d single -m single"
TEST_DEV=<dev>
SCRATCH_DEV_POOL=<dev-pool>

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests-nullblk.sh and zonefs-tests.sh from zonefs-tools were run
with no failures.

nullblk:
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified if dm-zoned successfully mounts without any
error.

-- progression
Emulated zoned device with zone size = 96M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Same profile as po2 zone size was used.

Many tests in xfstests for btrfs included dm-flakey and some tests
required dm-linear. As they are not supported at the moment for non
po2 devices, those **tests were excluded for non po2 devices**.

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests.sh from zonefs-tools were run with no failures.

nullblk:
A new section was added to cover non po2 devices:

section14()
{
       conv_pcnt=10
       zone_size=3
       zone_capacity=3
       max_open=${set_max_open}
       zbd_test_opts+=("-o ${max_open}")
}
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified that dm-zoned does not mount.

- Tools:
Some tools had to be updated to support non po2 devices. Once these
patches are accepted in the kernel, these tool updates will also be
upstreamed.
* btrfs-prog: https://github.com/Panky-codes/btrfs-progs/tree/remove-po2-btrfs
* blkzone: https://github.com/Panky-codes/util-linux/tree/remove-po2
* zonefs-tools: https://github.com/Panky-codes/zonefs-tools/tree/remove-po2

- Future work
To reduce the amount of changes and testing, support for DM was
excluded in this round of patches. The plan is to add support to F2FS
and DM in the forthcoming future.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test&id=6afcf6493578e77528abe65ab8b12f3e1c16749f
[1] https://lore.kernel.org/all/20220310094725.GA28499@lst.de/T/
[2] https://lore.kernel.org/all/20220315135245.eqf4tqngxxb7ymqa@unifi/

Changes since v1:
- Put the function declaration and its usage in the same commit (Bart)
- Remove bdev_zone_aligned function (Bart)
- Change the name from blk_queue_zone_aligned to blk_queue_is_zone_start
  (Damien)
- q is never null in from bdev_get_queue (Damien)
- Add condition during bringup and check for zsze == zcap for npo2
  drives (Damien)
- Rounddown operation should be made generic to work in 32 bits arch
  (bart)
- Add comments where generic calculation is directly used instead having
  special handling for po2 zone sizes (Hannes)
- Make the minimum zone size alignment requirement for btrfs to be 1M
  instead of BTRFS_STRIPE_LEN(David)

Changes since v2:
- Minor formatting changes

Changes since v3:
- Make superblock mirror align with the existing superblock log offsets
  (David)
- DM change return value and remove extra newline (Damien and Mike)
- Optimize null blk zone index lookup with shift for po2 zone size
  (Damien)

Luis Chamberlain (1):
  dm-zoned: ensure only power of 2 zone sizes are allowed

Pankaj Raghav (12):
  block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2
    zsze
  block: allow blk-zoned devices to have non-power-of-2 zone size
  nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
  nvmet: Allow ZNS target to support non-power_of_2 zone sizes
  btrfs: zoned: Cache superblock location in btrfs_zoned_device_info
  btrfs: zoned: Make sb_zone_number function non power of 2 compatible
  btrfs: zoned: use generic btrfs zone helpers to support npo2 zoned
    devices
  btrfs:zoned: make sb for npo2 zone devices align with sb log offsets
  btrfs: zoned: relax the alignment constraint for zoned devices
  zonefs: allow non power of 2 zoned devices
  null_blk: allow non power of 2 zoned devices
  null_blk: use zone_size_sects_shift for power of 2 zoned devices

 block/blk-core.c                  |   3 +-
 block/blk-zoned.c                 |  40 +++++--
 drivers/block/null_blk/main.c     |   5 +-
 drivers/block/null_blk/null_blk.h |   6 +
 drivers/block/null_blk/zoned.c    |  20 ++--
 drivers/md/dm-zone.c              |  12 ++
 drivers/nvme/host/zns.c           |  24 ++--
 drivers/nvme/target/zns.c         |   2 +-
 fs/btrfs/volumes.c                |  24 ++--
 fs/btrfs/zoned.c                  | 191 +++++++++++++++++++++---------
 fs/btrfs/zoned.h                  |  44 ++++++-
 fs/zonefs/super.c                 |   6 +-
 fs/zonefs/zonefs.h                |   1 -
 include/linux/blkdev.h            |  37 +++++-
 14 files changed, 303 insertions(+), 112 deletions(-)

-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [PATCH v4 01/13] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze
       [not found]   ` <CGME20220516133924eucas1p1817f306e3f2442088bf49ab513657cbe@eucas1p1.samsung.com>
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Pankaj Raghav,
	Luis Chamberlain, Hannes Reinecke

Adapt blkdev_nr_zones and blk_queue_zone_no function so that it can
also work for non-power-of-2 zone sizes.

As the existing deployments of zoned devices had power-of-2
assumption, power-of-2 optimized calculation is kept for those devices.

There are no direct hot paths modified and the changes just
introduce one new branch per call.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 block/blk-zoned.c      | 13 ++++++++++---
 include/linux/blkdev.h |  8 +++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 38cd840d8..140230134 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -111,16 +111,23 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock);
  * blkdev_nr_zones - Get number of zones
  * @disk:	Target gendisk
  *
- * Return the total number of zones of a zoned block device.  For a block
- * device without zone capabilities, the number of zones is always 0.
+ * Return the total number of zones of a zoned block device, including the
+ * eventual small last zone if present. For a block device without zone
+ * capabilities, the number of zones is always 0.
  */
 unsigned int blkdev_nr_zones(struct gendisk *disk)
 {
 	sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
+	sector_t capacity = get_capacity(disk);
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+
+	if (is_power_of_2(zone_sectors))
+		return (capacity + zone_sectors - 1) >>
+		       ilog2(zone_sectors);
+
+	return div64_u64(capacity + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1b24c1fb3..22fe512ee 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -675,9 +675,15 @@ static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
 static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 					     sector_t sector)
 {
+	sector_t zone_sectors = blk_queue_zone_sectors(q);
+
 	if (!blk_queue_is_zoned(q))
 		return 0;
-	return sector >> ilog2(q->limits.chunk_sectors);
+
+	if (is_power_of_2(zone_sectors))
+		return sector >> ilog2(zone_sectors);
+
+	return div64_u64(sector, zone_sectors);
 }
 
 static inline bool blk_queue_zone_is_seq(struct request_queue *q,
-- 
2.25.1


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

* [dm-devel] [PATCH v4 01/13] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, linux-nvme, Chris Mason, dm-devel, Alasdair Kergon,
	bvanassche, gost.dev, jonathan.derrick, Pankaj Raghav,
	Chaitanya Kulkarni, Josef Bacik, Jens Axboe, Keith Busch,
	matias.bjorling, Sagi Grimberg, linux-block, linux-kernel,
	Luis Chamberlain, linux-fsdevel, Johannes Thumshirn, linux-btrfs

Adapt blkdev_nr_zones and blk_queue_zone_no function so that it can
also work for non-power-of-2 zone sizes.

As the existing deployments of zoned devices had power-of-2
assumption, power-of-2 optimized calculation is kept for those devices.

There are no direct hot paths modified and the changes just
introduce one new branch per call.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 block/blk-zoned.c      | 13 ++++++++++---
 include/linux/blkdev.h |  8 +++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 38cd840d8..140230134 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -111,16 +111,23 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock);
  * blkdev_nr_zones - Get number of zones
  * @disk:	Target gendisk
  *
- * Return the total number of zones of a zoned block device.  For a block
- * device without zone capabilities, the number of zones is always 0.
+ * Return the total number of zones of a zoned block device, including the
+ * eventual small last zone if present. For a block device without zone
+ * capabilities, the number of zones is always 0.
  */
 unsigned int blkdev_nr_zones(struct gendisk *disk)
 {
 	sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
+	sector_t capacity = get_capacity(disk);
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+
+	if (is_power_of_2(zone_sectors))
+		return (capacity + zone_sectors - 1) >>
+		       ilog2(zone_sectors);
+
+	return div64_u64(capacity + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1b24c1fb3..22fe512ee 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -675,9 +675,15 @@ static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
 static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 					     sector_t sector)
 {
+	sector_t zone_sectors = blk_queue_zone_sectors(q);
+
 	if (!blk_queue_is_zoned(q))
 		return 0;
-	return sector >> ilog2(q->limits.chunk_sectors);
+
+	if (is_power_of_2(zone_sectors))
+		return sector >> ilog2(zone_sectors);
+
+	return div64_u64(sector, zone_sectors);
 }
 
 static inline bool blk_queue_zone_is_seq(struct request_queue *q,
-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [PATCH v4 02/13] block: allow blk-zoned devices to have non-power-of-2 zone size
       [not found]   ` <CGME20220516133925eucas1p1414fab2cfa7da1d6258315cbd33e1685@eucas1p1.samsung.com>
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Pankaj Raghav,
	Luis Chamberlain

Checking if a given sector is aligned to a zone is a common
operation that is performed for zoned devices. Add
blk_queue_is_zone_start helper to check for this instead of opencoding it
everywhere.

Convert the calculations on zone size to be generic instead of relying on
power_of_2 based logic in the block layer using the helpers wherever
possible.

The only hot path affected by this change for power_of_2 zoned devices
is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
used to optimize the calculation for po2 zone sizes. Note that the append
path cannot be accessed by direct raw access to the block device but only
through a filesystem abstraction.

Finally, allow non power of 2 zoned devices provided that their zone
capacity and zone size are equal. The main motivation to allow non
power_of_2 zoned device is to remove the unmapped LBA between zcap and
zsze for devices that cannot have a power_of_2 zcap.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 block/blk-core.c       |  3 +--
 block/blk-zoned.c      | 27 +++++++++++++++++++++------
 include/linux/blkdev.h | 22 ++++++++++++++++++++++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f305cb66c..b7051b7ea 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -634,8 +634,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
 		return BLK_STS_NOTSUPP;
 
 	/* The bio sector must point to the start of a sequential zone */
-	if (pos & (blk_queue_zone_sectors(q) - 1) ||
-	    !blk_queue_zone_is_seq(q, pos))
+	if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
 		return BLK_STS_IOERR;
 
 	/*
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 140230134..cfc2fb804 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -289,10 +289,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
 		return -EINVAL;
 
 	/* Check alignment (handle eventual smaller last zone) */
-	if (sector & (zone_sectors - 1))
+	if (!blk_queue_is_zone_start(q, sector))
 		return -EINVAL;
 
-	if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
+	if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
 		return -EINVAL;
 
 	/*
@@ -490,14 +490,29 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
 	 * smaller last zone.
 	 */
 	if (zone->start == 0) {
-		if (zone->len == 0 || !is_power_of_2(zone->len)) {
-			pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
-				disk->disk_name, zone->len);
+		if (zone->len == 0) {
+			pr_warn("%s: Invalid zone size",
+				disk->disk_name);
+			return -ENODEV;
+		}
+
+		/*
+		 * Don't allow zoned device with non power_of_2 zone size with
+		 * zone capacity less than zone size.
+		 */
+		if (!is_power_of_2(zone->len) &&
+		    zone->capacity < zone->len) {
+			pr_warn("%s: Invalid zoned size with non power of 2 zone size and zone capacity < zone size",
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		/*
+		 * Division is used to calculate nr_zones for both power_of_2
+		 * and non power_of_2 zone sizes as it is not in the hot path.
+		 */
+		args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
 	} else if (zone->start + args->zone_sectors < capacity) {
 		if (zone->len != args->zone_sectors) {
 			pr_warn("%s: Invalid zoned device with non constant zone size\n",
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 22fe512ee..32d7bd7b1 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -686,6 +686,22 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 	return div64_u64(sector, zone_sectors);
 }
 
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+	sector_t zone_sectors = blk_queue_zone_sectors(q);
+	u64 remainder = 0;
+
+	if (!blk_queue_is_zoned(q))
+		return false;
+
+	if (is_power_of_2(zone_sectors))
+		return IS_ALIGNED(sec, zone_sectors);
+
+	div64_u64_rem(sec, zone_sectors, &remainder);
+	/* if there is a remainder, then the sector is not aligned */
+	return remainder == 0;
+}
+
 static inline bool blk_queue_zone_is_seq(struct request_queue *q,
 					 sector_t sector)
 {
@@ -732,6 +748,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 {
 	return 0;
 }
+
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+	return false;
+}
+
 static inline unsigned int queue_max_open_zones(const struct request_queue *q)
 {
 	return 0;
-- 
2.25.1


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

* [dm-devel] [PATCH v4 02/13] block: allow blk-zoned devices to have non-power-of-2 zone size
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Pankaj Raghav,
	Luis Chamberlain, linux-kernel, linux-nvme, Josef Bacik,
	linux-block, linux-fsdevel, dm-devel, Alasdair Kergon,
	jonathan.derrick, Keith Busch, Johannes Thumshirn, linux-btrfs,
	Sagi Grimberg

Checking if a given sector is aligned to a zone is a common
operation that is performed for zoned devices. Add
blk_queue_is_zone_start helper to check for this instead of opencoding it
everywhere.

Convert the calculations on zone size to be generic instead of relying on
power_of_2 based logic in the block layer using the helpers wherever
possible.

The only hot path affected by this change for power_of_2 zoned devices
is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
used to optimize the calculation for po2 zone sizes. Note that the append
path cannot be accessed by direct raw access to the block device but only
through a filesystem abstraction.

Finally, allow non power of 2 zoned devices provided that their zone
capacity and zone size are equal. The main motivation to allow non
power_of_2 zoned device is to remove the unmapped LBA between zcap and
zsze for devices that cannot have a power_of_2 zcap.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 block/blk-core.c       |  3 +--
 block/blk-zoned.c      | 27 +++++++++++++++++++++------
 include/linux/blkdev.h | 22 ++++++++++++++++++++++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f305cb66c..b7051b7ea 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -634,8 +634,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
 		return BLK_STS_NOTSUPP;
 
 	/* The bio sector must point to the start of a sequential zone */
-	if (pos & (blk_queue_zone_sectors(q) - 1) ||
-	    !blk_queue_zone_is_seq(q, pos))
+	if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
 		return BLK_STS_IOERR;
 
 	/*
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 140230134..cfc2fb804 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -289,10 +289,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
 		return -EINVAL;
 
 	/* Check alignment (handle eventual smaller last zone) */
-	if (sector & (zone_sectors - 1))
+	if (!blk_queue_is_zone_start(q, sector))
 		return -EINVAL;
 
-	if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
+	if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
 		return -EINVAL;
 
 	/*
@@ -490,14 +490,29 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
 	 * smaller last zone.
 	 */
 	if (zone->start == 0) {
-		if (zone->len == 0 || !is_power_of_2(zone->len)) {
-			pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
-				disk->disk_name, zone->len);
+		if (zone->len == 0) {
+			pr_warn("%s: Invalid zone size",
+				disk->disk_name);
+			return -ENODEV;
+		}
+
+		/*
+		 * Don't allow zoned device with non power_of_2 zone size with
+		 * zone capacity less than zone size.
+		 */
+		if (!is_power_of_2(zone->len) &&
+		    zone->capacity < zone->len) {
+			pr_warn("%s: Invalid zoned size with non power of 2 zone size and zone capacity < zone size",
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		/*
+		 * Division is used to calculate nr_zones for both power_of_2
+		 * and non power_of_2 zone sizes as it is not in the hot path.
+		 */
+		args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
 	} else if (zone->start + args->zone_sectors < capacity) {
 		if (zone->len != args->zone_sectors) {
 			pr_warn("%s: Invalid zoned device with non constant zone size\n",
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 22fe512ee..32d7bd7b1 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -686,6 +686,22 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 	return div64_u64(sector, zone_sectors);
 }
 
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+	sector_t zone_sectors = blk_queue_zone_sectors(q);
+	u64 remainder = 0;
+
+	if (!blk_queue_is_zoned(q))
+		return false;
+
+	if (is_power_of_2(zone_sectors))
+		return IS_ALIGNED(sec, zone_sectors);
+
+	div64_u64_rem(sec, zone_sectors, &remainder);
+	/* if there is a remainder, then the sector is not aligned */
+	return remainder == 0;
+}
+
 static inline bool blk_queue_zone_is_seq(struct request_queue *q,
 					 sector_t sector)
 {
@@ -732,6 +748,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
 {
 	return 0;
 }
+
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+	return false;
+}
+
 static inline unsigned int queue_max_open_zones(const struct request_queue *q)
 {
 	return 0;
-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [PATCH v4 03/13] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
       [not found]   ` <CGME20220516133926eucas1p15c7ba425b67ce4ac824c6bd3263e2dd4@eucas1p1.samsung.com>
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Pankaj Raghav,
	Luis Chamberlain

Remove the condition which disallows non-power_of_2 zone size ZNS drive
to be updated and use generic method to calculate number of zones
instead of relying on log and shift based calculation on zone size.

The power_of_2 calculation has been replaced directly with generic
calculation without special handling. Both modified functions are not
used in hot paths, they are only used during initialization &
revalidation of the ZNS device.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/nvme/host/zns.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 9f81beb4d..65d2aa68a 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 	}
 
 	ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
-	if (!is_power_of_2(ns->zsze)) {
-		dev_warn(ns->ctrl->device,
-			"invalid zone size:%llu for namespace:%u\n",
-			ns->zsze, ns->head->ns_id);
-		status = -ENODEV;
-		goto free_data;
-	}
 
 	blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
@@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
 	const size_t min_bufsize = sizeof(struct nvme_zone_report) +
 				   sizeof(struct nvme_zone_descriptor);
 
+	/*
+	 * Division is used to calculate nr_zones with no special handling
+	 * for power of 2 zone sizes as this function is not invoked in a
+	 * hot path
+	 */
 	nr_zones = min_t(unsigned int, nr_zones,
-			 get_capacity(ns->disk) >> ilog2(ns->zsze));
+			 div64_u64(get_capacity(ns->disk), ns->zsze));
 
 	bufsize = sizeof(struct nvme_zone_report) +
 		nr_zones * sizeof(struct nvme_zone_descriptor);
@@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 	int ret, zone_idx = 0;
 	unsigned int nz, i;
 	size_t buflen;
+	u64 remainder = 0;
 
 	if (ns->head->ids.csi != NVME_CSI_ZNS)
 		return -EINVAL;
@@ -197,7 +196,14 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 	c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
 	c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
 
-	sector &= ~(ns->zsze - 1);
+	/*
+	 * rounddown the sector value to the nearest zone size. roundown macro
+	 * provided in math.h will not work for 32 bit architectures.
+	 * Division is used here with no special handling for power of 2
+	 * zone sizes as this function is not invoked in a hot path
+	 */
+	div64_u64_rem(sector, ns->zsze, &remainder);
+	sector -= remainder;
 	while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
 		memset(report, 0, buflen);
 
-- 
2.25.1


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

* [dm-devel] [PATCH v4 03/13] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Pankaj Raghav,
	Luis Chamberlain, linux-kernel, linux-nvme, Josef Bacik,
	linux-block, linux-fsdevel, dm-devel, Alasdair Kergon,
	jonathan.derrick, Keith Busch, Johannes Thumshirn, linux-btrfs,
	Sagi Grimberg

Remove the condition which disallows non-power_of_2 zone size ZNS drive
to be updated and use generic method to calculate number of zones
instead of relying on log and shift based calculation on zone size.

The power_of_2 calculation has been replaced directly with generic
calculation without special handling. Both modified functions are not
used in hot paths, they are only used during initialization &
revalidation of the ZNS device.

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/nvme/host/zns.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 9f81beb4d..65d2aa68a 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 	}
 
 	ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
-	if (!is_power_of_2(ns->zsze)) {
-		dev_warn(ns->ctrl->device,
-			"invalid zone size:%llu for namespace:%u\n",
-			ns->zsze, ns->head->ns_id);
-		status = -ENODEV;
-		goto free_data;
-	}
 
 	blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
@@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
 	const size_t min_bufsize = sizeof(struct nvme_zone_report) +
 				   sizeof(struct nvme_zone_descriptor);
 
+	/*
+	 * Division is used to calculate nr_zones with no special handling
+	 * for power of 2 zone sizes as this function is not invoked in a
+	 * hot path
+	 */
 	nr_zones = min_t(unsigned int, nr_zones,
-			 get_capacity(ns->disk) >> ilog2(ns->zsze));
+			 div64_u64(get_capacity(ns->disk), ns->zsze));
 
 	bufsize = sizeof(struct nvme_zone_report) +
 		nr_zones * sizeof(struct nvme_zone_descriptor);
@@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 	int ret, zone_idx = 0;
 	unsigned int nz, i;
 	size_t buflen;
+	u64 remainder = 0;
 
 	if (ns->head->ids.csi != NVME_CSI_ZNS)
 		return -EINVAL;
@@ -197,7 +196,14 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 	c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
 	c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
 
-	sector &= ~(ns->zsze - 1);
+	/*
+	 * rounddown the sector value to the nearest zone size. roundown macro
+	 * provided in math.h will not work for 32 bit architectures.
+	 * Division is used here with no special handling for power of 2
+	 * zone sizes as this function is not invoked in a hot path
+	 */
+	div64_u64_rem(sector, ns->zsze, &remainder);
+	sector -= remainder;
 	while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
 		memset(report, 0, buflen);
 
-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [PATCH v4 04/13] nvmet: Allow ZNS target to support non-power_of_2 zone sizes
       [not found]   ` <CGME20220516133927eucas1p1bab57e07c14c1194705e254afdd5d346@eucas1p1.samsung.com>
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Pankaj Raghav,
	Hannes Reinecke, Luis Chamberlain

A generic bdev_zone_no helper is added to calculate zone number for a given
sector in a block device. This helper internally uses blk_queue_zone_no to
find the zone number.

Use the helper bdev_zone_no() to calculate nr of zones. This let's us
make modifications to the math if needed in one place and adds now
support for npo2 zone devices.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/nvme/target/zns.c | 2 +-
 include/linux/blkdev.h    | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 82b61acf7..5516dd6cc 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -242,7 +242,7 @@ static unsigned long nvmet_req_nr_zones_from_slba(struct nvmet_req *req)
 	unsigned int sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
 
 	return blkdev_nr_zones(req->ns->bdev->bd_disk) -
-		(sect >> ilog2(bdev_zone_sectors(req->ns->bdev)));
+	       bdev_zone_no(req->ns->bdev, sect);
 }
 
 static unsigned long get_nr_zones_from_buf(struct nvmet_req *req, u32 bufsize)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 32d7bd7b1..967790f51 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1370,6 +1370,13 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
 	return 0;
 }
 
+static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec)
+{
+	struct request_queue *q = bdev_get_queue(bdev);
+
+	return blk_queue_zone_no(q, sec);
+}
+
 static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
 {
 	struct request_queue *q = bdev_get_queue(bdev);
-- 
2.25.1


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

* [dm-devel] [PATCH v4 04/13] nvmet: Allow ZNS target to support non-power_of_2 zone sizes
@ 2022-05-16 13:39       ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 13:39 UTC (permalink / raw)
  To: axboe, naohiro.aota, damien.lemoal, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, linux-nvme, Chris Mason, dm-devel, Alasdair Kergon,
	bvanassche, gost.dev, jonathan.derrick, Pankaj Raghav,
	Chaitanya Kulkarni, Josef Bacik, Jens Axboe, Keith Busch,
	matias.bjorling, Sagi Grimberg, linux-block, linux-kernel,
	Luis Chamberlain, linux-fsdevel, Johannes Thumshirn, linux-btrfs

A generic bdev_zone_no helper is added to calculate zone number for a given
sector in a block device. This helper internally uses blk_queue_zone_no to
find the zone number.

Use the helper bdev_zone_no() to calculate nr of zones. This let's us
make modifications to the math if needed in one place and adds now
support for npo2 zone devices.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/nvme/target/zns.c | 2 +-
 include/linux/blkdev.h    | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 82b61acf7..5516dd6cc 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -242,7 +242,7 @@ static unsigned long nvmet_req_nr_zones_from_slba(struct nvmet_req *req)
 	unsigned int sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
 
 	return blkdev_nr_zones(req->ns->bdev->bd_disk) -
-		(sect >> ilog2(bdev_zone_sectors(req->ns->bdev)));
+	       bdev_zone_no(req->ns->bdev, sect);
 }
 
 static unsigned long get_nr_zones_from_buf(struct nvmet_req *req, u32 bufsize)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 32d7bd7b1..967790f51 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1370,6 +1370,13 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
 	return 0;
 }
 
+static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec)
+{
+	struct request_queue *q = bdev_get_queue(bdev);
+
+	return blk_queue_zone_no(q, sec);
+}
+
 static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
 {
 	struct request_queue *q = bdev_get_queue(bdev);
-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [PATCH v4 01/13] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze
  2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
@ 2022-05-16 13:54         ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 13:54 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Luis Chamberlain,
	Hannes Reinecke

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Adapt blkdev_nr_zones and blk_queue_zone_no function so that it can
> also work for non-power-of-2 zone sizes.
> 
> As the existing deployments of zoned devices had power-of-2
> assumption, power-of-2 optimized calculation is kept for those devices.
> 
> There are no direct hot paths modified and the changes just
> introduce one new branch per call.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  block/blk-zoned.c      | 13 ++++++++++---
>  include/linux/blkdev.h |  8 +++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 38cd840d8..140230134 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -111,16 +111,23 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock);
>   * blkdev_nr_zones - Get number of zones
>   * @disk:	Target gendisk
>   *
> - * Return the total number of zones of a zoned block device.  For a block
> - * device without zone capabilities, the number of zones is always 0.
> + * Return the total number of zones of a zoned block device, including the
> + * eventual small last zone if present. For a block device without zone
> + * capabilities, the number of zones is always 0.
>   */
>  unsigned int blkdev_nr_zones(struct gendisk *disk)
>  {
>  	sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
> +	sector_t capacity = get_capacity(disk);
>  
>  	if (!blk_queue_is_zoned(disk->queue))
>  		return 0;
> -	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
> +
> +	if (is_power_of_2(zone_sectors))
> +		return (capacity + zone_sectors - 1) >>
> +		       ilog2(zone_sectors);

Why the line break here ? This fits on one line, no ?

> +
> +	return div64_u64(capacity + zone_sectors - 1, zone_sectors);
>  }
>  EXPORT_SYMBOL_GPL(blkdev_nr_zones);
>  
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 1b24c1fb3..22fe512ee 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -675,9 +675,15 @@ static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
>  static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  					     sector_t sector)
>  {
> +	sector_t zone_sectors = blk_queue_zone_sectors(q);
> +
>  	if (!blk_queue_is_zoned(q))
>  		return 0;
> -	return sector >> ilog2(q->limits.chunk_sectors);
> +
> +	if (is_power_of_2(zone_sectors))
> +		return sector >> ilog2(zone_sectors);
> +
> +	return div64_u64(sector, zone_sectors);
>  }
>  
>  static inline bool blk_queue_zone_is_seq(struct request_queue *q,


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 01/13] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze
@ 2022-05-16 13:54         ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 13:54 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Luis Chamberlain,
	linux-kernel, linux-nvme, Josef Bacik, linux-block,
	linux-fsdevel, dm-devel, Alasdair Kergon, jonathan.derrick,
	Keith Busch, Johannes Thumshirn, linux-btrfs, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Adapt blkdev_nr_zones and blk_queue_zone_no function so that it can
> also work for non-power-of-2 zone sizes.
> 
> As the existing deployments of zoned devices had power-of-2
> assumption, power-of-2 optimized calculation is kept for those devices.
> 
> There are no direct hot paths modified and the changes just
> introduce one new branch per call.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  block/blk-zoned.c      | 13 ++++++++++---
>  include/linux/blkdev.h |  8 +++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 38cd840d8..140230134 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -111,16 +111,23 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock);
>   * blkdev_nr_zones - Get number of zones
>   * @disk:	Target gendisk
>   *
> - * Return the total number of zones of a zoned block device.  For a block
> - * device without zone capabilities, the number of zones is always 0.
> + * Return the total number of zones of a zoned block device, including the
> + * eventual small last zone if present. For a block device without zone
> + * capabilities, the number of zones is always 0.
>   */
>  unsigned int blkdev_nr_zones(struct gendisk *disk)
>  {
>  	sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
> +	sector_t capacity = get_capacity(disk);
>  
>  	if (!blk_queue_is_zoned(disk->queue))
>  		return 0;
> -	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
> +
> +	if (is_power_of_2(zone_sectors))
> +		return (capacity + zone_sectors - 1) >>
> +		       ilog2(zone_sectors);

Why the line break here ? This fits on one line, no ?

> +
> +	return div64_u64(capacity + zone_sectors - 1, zone_sectors);
>  }
>  EXPORT_SYMBOL_GPL(blkdev_nr_zones);
>  
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 1b24c1fb3..22fe512ee 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -675,9 +675,15 @@ static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
>  static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  					     sector_t sector)
>  {
> +	sector_t zone_sectors = blk_queue_zone_sectors(q);
> +
>  	if (!blk_queue_is_zoned(q))
>  		return 0;
> -	return sector >> ilog2(q->limits.chunk_sectors);
> +
> +	if (is_power_of_2(zone_sectors))
> +		return sector >> ilog2(zone_sectors);
> +
> +	return div64_u64(sector, zone_sectors);
>  }
>  
>  static inline bool blk_queue_zone_is_seq(struct request_queue *q,


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [PATCH v4 02/13] block: allow blk-zoned devices to have non-power-of-2 zone size
  2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
@ 2022-05-16 14:00         ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:00 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-btrfs, linux-kernel, jonathan.derrick, bvanassche,
	Keith Busch, gost.dev, linux-nvme, Johannes Thumshirn,
	Josef Bacik, linux-block, Alasdair Kergon, matias.bjorling,
	Jens Axboe, Sagi Grimberg, dm-devel, jiangbo.365,
	Chaitanya Kulkarni, linux-fsdevel, Chris Mason, Luis Chamberlain

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Checking if a given sector is aligned to a zone is a common
> operation that is performed for zoned devices. Add
> blk_queue_is_zone_start helper to check for this instead of opencoding it
> everywhere.
> 
> Convert the calculations on zone size to be generic instead of relying on
> power_of_2 based logic in the block layer using the helpers wherever
> possible.
> 
> The only hot path affected by this change for power_of_2 zoned devices
> is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
> used to optimize the calculation for po2 zone sizes. Note that the append
> path cannot be accessed by direct raw access to the block device but only
> through a filesystem abstraction.
> 
> Finally, allow non power of 2 zoned devices provided that their zone
> capacity and zone size are equal. The main motivation to allow non
> power_of_2 zoned device is to remove the unmapped LBA between zcap and
> zsze for devices that cannot have a power_of_2 zcap.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  block/blk-core.c       |  3 +--
>  block/blk-zoned.c      | 27 +++++++++++++++++++++------
>  include/linux/blkdev.h | 22 ++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f305cb66c..b7051b7ea 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -634,8 +634,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
>  		return BLK_STS_NOTSUPP;
>  
>  	/* The bio sector must point to the start of a sequential zone */
> -	if (pos & (blk_queue_zone_sectors(q) - 1) ||
> -	    !blk_queue_zone_is_seq(q, pos))
> +	if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
>  		return BLK_STS_IOERR;
>  
>  	/*
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 140230134..cfc2fb804 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -289,10 +289,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
>  		return -EINVAL;
>  
>  	/* Check alignment (handle eventual smaller last zone) */
> -	if (sector & (zone_sectors - 1))
> +	if (!blk_queue_is_zone_start(q, sector))
>  		return -EINVAL;
>  
> -	if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
> +	if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
>  		return -EINVAL;
>  
>  	/*
> @@ -490,14 +490,29 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
>  	 * smaller last zone.
>  	 */
>  	if (zone->start == 0) {
> -		if (zone->len == 0 || !is_power_of_2(zone->len)) {
> -			pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
> -				disk->disk_name, zone->len);
> +		if (zone->len == 0) {
> +			pr_warn("%s: Invalid zone size",
> +				disk->disk_name);

This fits on one line, no ?

> +			return -ENODEV;
> +		}
> +
> +		/*
> +		 * Don't allow zoned device with non power_of_2 zone size with
> +		 * zone capacity less than zone size.
> +		 */
> +		if (!is_power_of_2(zone->len) &&
> +		    zone->capacity < zone->len) {
> +			pr_warn("%s: Invalid zoned size with non power of 2 zone size and zone capacity < zone size",
> +				disk->disk_name);

Very long... What about:

pr_warn("%s: Invalid zone capacity for non power of 2 zone size",
	disk->disk_name);

>  			return -ENODEV;
>  		}
>  
>  		args->zone_sectors = zone->len;
> -		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
> +		/*
> +		 * Division is used to calculate nr_zones for both power_of_2
> +		 * and non power_of_2 zone sizes as it is not in the hot path.
> +		 */

This comment is not very useful.

> +		args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
>  	} else if (zone->start + args->zone_sectors < capacity) {
>  		if (zone->len != args->zone_sectors) {
>  			pr_warn("%s: Invalid zoned device with non constant zone size\n",
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 22fe512ee..32d7bd7b1 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -686,6 +686,22 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  	return div64_u64(sector, zone_sectors);
>  }
>  
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> +	sector_t zone_sectors = blk_queue_zone_sectors(q);
> +	u64 remainder = 0;
> +
> +	if (!blk_queue_is_zoned(q))
> +		return false;
> +
> +	if (is_power_of_2(zone_sectors))
> +		return IS_ALIGNED(sec, zone_sectors);
> +
> +	div64_u64_rem(sec, zone_sectors, &remainder);
> +	/* if there is a remainder, then the sector is not aligned */

Hmmm... Fairly obvious. Not sure this comment is useful.

> +	return remainder == 0;
> +}
> +
>  static inline bool blk_queue_zone_is_seq(struct request_queue *q,
>  					 sector_t sector)
>  {
> @@ -732,6 +748,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  {
>  	return 0;
>  }
> +
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> +	return false;
> +}
> +
>  static inline unsigned int queue_max_open_zones(const struct request_queue *q)
>  {
>  	return 0;


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 02/13] block: allow blk-zoned devices to have non-power-of-2 zone size
@ 2022-05-16 14:00         ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:00 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Luis Chamberlain,
	linux-kernel, linux-nvme, Josef Bacik, linux-block,
	linux-fsdevel, dm-devel, Alasdair Kergon, jonathan.derrick,
	Keith Busch, Johannes Thumshirn, linux-btrfs, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Checking if a given sector is aligned to a zone is a common
> operation that is performed for zoned devices. Add
> blk_queue_is_zone_start helper to check for this instead of opencoding it
> everywhere.
> 
> Convert the calculations on zone size to be generic instead of relying on
> power_of_2 based logic in the block layer using the helpers wherever
> possible.
> 
> The only hot path affected by this change for power_of_2 zoned devices
> is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
> used to optimize the calculation for po2 zone sizes. Note that the append
> path cannot be accessed by direct raw access to the block device but only
> through a filesystem abstraction.
> 
> Finally, allow non power of 2 zoned devices provided that their zone
> capacity and zone size are equal. The main motivation to allow non
> power_of_2 zoned device is to remove the unmapped LBA between zcap and
> zsze for devices that cannot have a power_of_2 zcap.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  block/blk-core.c       |  3 +--
>  block/blk-zoned.c      | 27 +++++++++++++++++++++------
>  include/linux/blkdev.h | 22 ++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f305cb66c..b7051b7ea 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -634,8 +634,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
>  		return BLK_STS_NOTSUPP;
>  
>  	/* The bio sector must point to the start of a sequential zone */
> -	if (pos & (blk_queue_zone_sectors(q) - 1) ||
> -	    !blk_queue_zone_is_seq(q, pos))
> +	if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
>  		return BLK_STS_IOERR;
>  
>  	/*
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 140230134..cfc2fb804 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -289,10 +289,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
>  		return -EINVAL;
>  
>  	/* Check alignment (handle eventual smaller last zone) */
> -	if (sector & (zone_sectors - 1))
> +	if (!blk_queue_is_zone_start(q, sector))
>  		return -EINVAL;
>  
> -	if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
> +	if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
>  		return -EINVAL;
>  
>  	/*
> @@ -490,14 +490,29 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
>  	 * smaller last zone.
>  	 */
>  	if (zone->start == 0) {
> -		if (zone->len == 0 || !is_power_of_2(zone->len)) {
> -			pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
> -				disk->disk_name, zone->len);
> +		if (zone->len == 0) {
> +			pr_warn("%s: Invalid zone size",
> +				disk->disk_name);

This fits on one line, no ?

> +			return -ENODEV;
> +		}
> +
> +		/*
> +		 * Don't allow zoned device with non power_of_2 zone size with
> +		 * zone capacity less than zone size.
> +		 */
> +		if (!is_power_of_2(zone->len) &&
> +		    zone->capacity < zone->len) {
> +			pr_warn("%s: Invalid zoned size with non power of 2 zone size and zone capacity < zone size",
> +				disk->disk_name);

Very long... What about:

pr_warn("%s: Invalid zone capacity for non power of 2 zone size",
	disk->disk_name);

>  			return -ENODEV;
>  		}
>  
>  		args->zone_sectors = zone->len;
> -		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
> +		/*
> +		 * Division is used to calculate nr_zones for both power_of_2
> +		 * and non power_of_2 zone sizes as it is not in the hot path.
> +		 */

This comment is not very useful.

> +		args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
>  	} else if (zone->start + args->zone_sectors < capacity) {
>  		if (zone->len != args->zone_sectors) {
>  			pr_warn("%s: Invalid zoned device with non constant zone size\n",
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 22fe512ee..32d7bd7b1 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -686,6 +686,22 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  	return div64_u64(sector, zone_sectors);
>  }
>  
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> +	sector_t zone_sectors = blk_queue_zone_sectors(q);
> +	u64 remainder = 0;
> +
> +	if (!blk_queue_is_zoned(q))
> +		return false;
> +
> +	if (is_power_of_2(zone_sectors))
> +		return IS_ALIGNED(sec, zone_sectors);
> +
> +	div64_u64_rem(sec, zone_sectors, &remainder);
> +	/* if there is a remainder, then the sector is not aligned */

Hmmm... Fairly obvious. Not sure this comment is useful.

> +	return remainder == 0;
> +}
> +
>  static inline bool blk_queue_zone_is_seq(struct request_queue *q,
>  					 sector_t sector)
>  {
> @@ -732,6 +748,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>  {
>  	return 0;
>  }
> +
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> +	return false;
> +}
> +
>  static inline unsigned int queue_max_open_zones(const struct request_queue *q)
>  {
>  	return 0;


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 03/13] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
  2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
@ 2022-05-16 14:02         ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:02 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, Luis Chamberlain,
	linux-kernel, linux-nvme, Josef Bacik, linux-block,
	linux-fsdevel, dm-devel, Alasdair Kergon, jonathan.derrick,
	Keith Busch, Johannes Thumshirn, linux-btrfs, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Remove the condition which disallows non-power_of_2 zone size ZNS drive
> to be updated and use generic method to calculate number of zones
> instead of relying on log and shift based calculation on zone size.
> 
> The power_of_2 calculation has been replaced directly with generic
> calculation without special handling. Both modified functions are not
> used in hot paths, they are only used during initialization &
> revalidation of the ZNS device.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  drivers/nvme/host/zns.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> index 9f81beb4d..65d2aa68a 100644
> --- a/drivers/nvme/host/zns.c
> +++ b/drivers/nvme/host/zns.c
> @@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
>  	}
>  
>  	ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
> -	if (!is_power_of_2(ns->zsze)) {
> -		dev_warn(ns->ctrl->device,
> -			"invalid zone size:%llu for namespace:%u\n",
> -			ns->zsze, ns->head->ns_id);
> -		status = -ENODEV;
> -		goto free_data;
> -	}
>  
>  	blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> @@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
>  	const size_t min_bufsize = sizeof(struct nvme_zone_report) +
>  				   sizeof(struct nvme_zone_descriptor);
>  
> +	/*
> +	 * Division is used to calculate nr_zones with no special handling
> +	 * for power of 2 zone sizes as this function is not invoked in a
> +	 * hot path
> +	 */

Comment not very useful.

>  	nr_zones = min_t(unsigned int, nr_zones,
> -			 get_capacity(ns->disk) >> ilog2(ns->zsze));
> +			 div64_u64(get_capacity(ns->disk), ns->zsze));
>  
>  	bufsize = sizeof(struct nvme_zone_report) +
>  		nr_zones * sizeof(struct nvme_zone_descriptor);
> @@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
>  	int ret, zone_idx = 0;
>  	unsigned int nz, i;
>  	size_t buflen;
> +	u64 remainder = 0;
>  
>  	if (ns->head->ids.csi != NVME_CSI_ZNS)
>  		return -EINVAL;
> @@ -197,7 +196,14 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
>  	c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
>  	c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
>  
> -	sector &= ~(ns->zsze - 1);
> +	/*
> +	 * rounddown the sector value to the nearest zone size. roundown macro
> +	 * provided in math.h will not work for 32 bit architectures.
> +	 * Division is used here with no special handling for power of 2
> +	 * zone sizes as this function is not invoked in a hot path
> +	 */

Please simplify this to:

	/* Round down the sector value to the nearest zone start */

> +	div64_u64_rem(sector, ns->zsze, &remainder);
> +	sector -= remainder;
>  	while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
>  		memset(report, 0, buflen);
>  


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 03/13] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
@ 2022-05-16 14:02         ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:02 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-block, Chaitanya Kulkarni, bvanassche, dm-devel, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, Jens Axboe, Chris Mason,
	Luis Chamberlain, linux-btrfs, Alasdair Kergon, jonathan.derrick,
	linux-fsdevel, Josef Bacik, Johannes Thumshirn, Keith Busch,
	matias.bjorling, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
> Remove the condition which disallows non-power_of_2 zone size ZNS drive
> to be updated and use generic method to calculate number of zones
> instead of relying on log and shift based calculation on zone size.
> 
> The power_of_2 calculation has been replaced directly with generic
> calculation without special handling. Both modified functions are not
> used in hot paths, they are only used during initialization &
> revalidation of the ZNS device.
> 
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  drivers/nvme/host/zns.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> index 9f81beb4d..65d2aa68a 100644
> --- a/drivers/nvme/host/zns.c
> +++ b/drivers/nvme/host/zns.c
> @@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
>  	}
>  
>  	ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
> -	if (!is_power_of_2(ns->zsze)) {
> -		dev_warn(ns->ctrl->device,
> -			"invalid zone size:%llu for namespace:%u\n",
> -			ns->zsze, ns->head->ns_id);
> -		status = -ENODEV;
> -		goto free_data;
> -	}
>  
>  	blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> @@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
>  	const size_t min_bufsize = sizeof(struct nvme_zone_report) +
>  				   sizeof(struct nvme_zone_descriptor);
>  
> +	/*
> +	 * Division is used to calculate nr_zones with no special handling
> +	 * for power of 2 zone sizes as this function is not invoked in a
> +	 * hot path
> +	 */

Comment not very useful.

>  	nr_zones = min_t(unsigned int, nr_zones,
> -			 get_capacity(ns->disk) >> ilog2(ns->zsze));
> +			 div64_u64(get_capacity(ns->disk), ns->zsze));
>  
>  	bufsize = sizeof(struct nvme_zone_report) +
>  		nr_zones * sizeof(struct nvme_zone_descriptor);
> @@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
>  	int ret, zone_idx = 0;
>  	unsigned int nz, i;
>  	size_t buflen;
> +	u64 remainder = 0;
>  
>  	if (ns->head->ids.csi != NVME_CSI_ZNS)
>  		return -EINVAL;
> @@ -197,7 +196,14 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
>  	c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
>  	c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
>  
> -	sector &= ~(ns->zsze - 1);
> +	/*
> +	 * rounddown the sector value to the nearest zone size. roundown macro
> +	 * provided in math.h will not work for 32 bit architectures.
> +	 * Division is used here with no special handling for power of 2
> +	 * zone sizes as this function is not invoked in a hot path
> +	 */

Please simplify this to:

	/* Round down the sector value to the nearest zone start */

> +	div64_u64_rem(sector, ns->zsze, &remainder);
> +	sector -= remainder;
>  	while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
>  		memset(report, 0, buflen);
>  


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-16 13:39   ` [dm-devel] " Pankaj Raghav
@ 2022-05-16 14:15     ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:15 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: jiangbo.365, Jens Axboe, Chaitanya Kulkarni, bvanassche,
	Chris Mason, matias.bjorling, gost.dev, linux-kernel, linux-nvme,
	Josef Bacik, linux-block, linux-fsdevel, dm-devel,
	Alasdair Kergon, jonathan.derrick, Keith Busch,
	Johannes Thumshirn, linux-btrfs, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
[...]
> - Patchset description:
> This patchset aims at adding support to non power of 2 zoned devices in
> the block layer, nvme layer, null blk and adds support to btrfs and
> zonefs.
> 
> This round of patches **will not** support DM layer for non
> power of 2 zoned devices. More about this in the future work section.
> 
> Patches 1-2 deals with removing the po2 constraint from the
> block layer.
> 
> Patches 3-4 deals with removing the constraint from nvme zns.
> 
> Patches 5-9 adds support to btrfs for non po2 zoned devices.
> 
> Patch 10 removes the po2 constraint in ZoneFS
> 
> Patch 11-12 removes the po2 contraint in null blk
> 
> Patches 13 adds conditions to not allow non power of 2 devices in
> DM.


Not sure what is going on but I only got the first 4 patches and I do not see
the remaining patches on the lists anywhere.


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-16 14:15     ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-16 14:15 UTC (permalink / raw)
  To: Pankaj Raghav, axboe, naohiro.aota, Johannes.Thumshirn, snitzer,
	dsterba, jaegeuk, hch
  Cc: linux-block, Chaitanya Kulkarni, bvanassche, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, Jens Axboe, Chris Mason,
	dm-devel, linux-btrfs, Alasdair Kergon, jonathan.derrick,
	linux-fsdevel, Josef Bacik, Johannes Thumshirn, Keith Busch,
	matias.bjorling, Sagi Grimberg

On 2022/05/16 15:39, Pankaj Raghav wrote:
[...]
> - Patchset description:
> This patchset aims at adding support to non power of 2 zoned devices in
> the block layer, nvme layer, null blk and adds support to btrfs and
> zonefs.
> 
> This round of patches **will not** support DM layer for non
> power of 2 zoned devices. More about this in the future work section.
> 
> Patches 1-2 deals with removing the po2 constraint from the
> block layer.
> 
> Patches 3-4 deals with removing the constraint from nvme zns.
> 
> Patches 5-9 adds support to btrfs for non po2 zoned devices.
> 
> Patch 10 removes the po2 constraint in ZoneFS
> 
> Patch 11-12 removes the po2 contraint in null blk
> 
> Patches 13 adds conditions to not allow non power of 2 devices in
> DM.


Not sure what is going on but I only got the first 4 patches and I do not see
the remaining patches on the lists anywhere.


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-16 14:15     ` Damien Le Moal
  (?)
@ 2022-05-16 14:33     ` notnand
  -1 siblings, 0 replies; 56+ messages in thread
From: notnand @ 2022-05-16 14:33 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: jiangbo.365, linux-nvme, Chris Mason, dm-devel,
	Christoph Hellwig, Alasdair Kergon, Naohiro Aota,
	Bart Van Assche, gost.dev, jonathan.derrick, Pankaj Raghav,
	Chaitanya Kulkarni, snitzer, Josef Bacik, Jens Axboe, dsterba,
	jaegeuk, Matias Bjørling, Sagi Grimberg, Jens Axboe,
	linux-block, Johannes Thumshirn, linux-kernel, Keith Busch,
	linux-fsdevel, Johannes Thumshirn, linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 1256 bytes --]

My mail server got messed up for some reason because of the number of
people in cc. I will send the whole patch series again in some time as I am
traveling now. Sorry about that.

On Mon, 16 May 2022, 16:20 Damien Le Moal, <damien.lemoal@opensource.wdc.com>
wrote:

> On 2022/05/16 15:39, Pankaj Raghav wrote:
> [...]
> > - Patchset description:
> > This patchset aims at adding support to non power of 2 zoned devices in
> > the block layer, nvme layer, null blk and adds support to btrfs and
> > zonefs.
> >
> > This round of patches **will not** support DM layer for non
> > power of 2 zoned devices. More about this in the future work section.
> >
> > Patches 1-2 deals with removing the po2 constraint from the
> > block layer.
> >
> > Patches 3-4 deals with removing the constraint from nvme zns.
> >
> > Patches 5-9 adds support to btrfs for non po2 zoned devices.
> >
> > Patch 10 removes the po2 constraint in ZoneFS
> >
> > Patch 11-12 removes the po2 contraint in null blk
> >
> > Patches 13 adds conditions to not allow non power of 2 devices in
> > DM.
>
>
> Not sure what is going on but I only got the first 4 patches and I do not
> see
> the remaining patches on the lists anywhere.
>
>
> --
> Damien Le Moal
> Western Digital Research
>
>

[-- Attachment #1.2: Type: text/html, Size: 1714 bytes --]

[-- Attachment #2: Type: text/plain, Size: 98 bytes --]

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20 17:18                           ` David Sterba
@ 2022-05-23  8:25                             ` Pankaj Raghav
  -1 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-23  8:25 UTC (permalink / raw)
  To: dsterba, Damien Le Moal, Javier González, Hannes Reinecke,
	Johannes Thumshirn, Mike Snitzer, Christoph Hellwig,
	Luis Chamberlain, Theodore Ts'o, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim,
	Keith Busch, Adam Manzanares

On 5/20/22 19:18, David Sterba wrote:
>> I see that many people in the community feel it is better to target the
>> dm layer for the initial support of npo2 devices. I can give it a shot
>> and maintain a native out-of-tree support for FSs for npo2 devices and
>> merge it upstream as we see fit later.
> 
> Some of the changes from your patchset are cleanups or abstracting the
> alignment and zone calculations, so this can be merged to minimize the
> out of tree code.
Sounds good. I will send it separately. Thanks.

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-23  8:25                             ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-23  8:25 UTC (permalink / raw)
  To: dsterba, Damien Le Moal, Javier González, Hannes Reinecke,
	Johannes Thumshirn, Mike Snitzer, Christoph Hellwig,
	Luis Chamberlain, Theodore Ts'o, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim,
	Keith Busch, Adam Manzanares

On 5/20/22 19:18, David Sterba wrote:
>> I see that many people in the community feel it is better to target the
>> dm layer for the initial support of npo2 devices. I can give it a shot
>> and maintain a native out-of-tree support for FSs for npo2 devices and
>> merge it upstream as we see fit later.
> 
> Some of the changes from your patchset are cleanups or abstracting the
> alignment and zone calculations, so this can be merged to minimize the
> out of tree code.
Sounds good. I will send it separately. Thanks.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  9:30                         ` Pankaj Raghav
@ 2022-05-20 17:18                           ` David Sterba
  -1 siblings, 0 replies; 56+ messages in thread
From: David Sterba @ 2022-05-20 17:18 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Damien Le Moal, Javier González, Hannes Reinecke,
	Johannes Thumshirn, Mike Snitzer, Christoph Hellwig,
	Luis Chamberlain, Theodore Ts'o, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim,
	Keith Busch, Adam Manzanares

On Fri, May 20, 2022 at 11:30:09AM +0200, Pankaj Raghav wrote:
> On 5/20/22 08:41, Damien Le Moal wrote:
> >> Note that for F2FS there is no blocker. Jaegeuk picked the initial
> >> patches, and he agreed to add native support.
> > 
> > And until that is done, f2fs will not work with these new !po2 devices...
> > Having the new dm will avoid that support fragmentation which I personally
> > really dislike. With the new dm, we can keep support for *all* zoned block
> > devices, albeit needing a different setup depending on the device. That is
> > not nice at all but at least there is a way to make things work continuously.
> 
> I see that many people in the community feel it is better to target the
> dm layer for the initial support of npo2 devices. I can give it a shot
> and maintain a native out-of-tree support for FSs for npo2 devices and
> merge it upstream as we see fit later.

Some of the changes from your patchset are cleanups or abstracting the
alignment and zone calculations, so this can be merged to minimize the
out of tree code.

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20 17:18                           ` David Sterba
  0 siblings, 0 replies; 56+ messages in thread
From: David Sterba @ 2022-05-20 17:18 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Mike Snitzer, jiangbo.365, linux-nvme, dsterba, dm-devel,
	Adam Manzanares, Christoph Hellwig, pankydev8, gost.dev,
	Damien Le Moal, linux-block, Keith Busch, Jaegeuk Kim, axboe,
	Theodore Ts'o, Johannes Thumshirn, linux-kernel,
	Luis Chamberlain, linux-fsdevel, Javier González,
	linux-btrfs

On Fri, May 20, 2022 at 11:30:09AM +0200, Pankaj Raghav wrote:
> On 5/20/22 08:41, Damien Le Moal wrote:
> >> Note that for F2FS there is no blocker. Jaegeuk picked the initial
> >> patches, and he agreed to add native support.
> > 
> > And until that is done, f2fs will not work with these new !po2 devices...
> > Having the new dm will avoid that support fragmentation which I personally
> > really dislike. With the new dm, we can keep support for *all* zoned block
> > devices, albeit needing a different setup depending on the device. That is
> > not nice at all but at least there is a way to make things work continuously.
> 
> I see that many people in the community feel it is better to target the
> dm layer for the initial support of npo2 devices. I can give it a shot
> and maintain a native out-of-tree support for FSs for npo2 devices and
> merge it upstream as we see fit later.

Some of the changes from your patchset are cleanups or abstracting the
alignment and zone calculations, so this can be merged to minimize the
out of tree code.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
       [not found]                       ` <CGME20220520101610eucas1p1822ca6014e2a1d55ae74476f83c4de1d@eucas1p1.samsung.com>
@ 2022-05-20 10:16                           ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20 10:16 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Hannes Reinecke, Damien Le Moal, Luis Chamberlain,
	Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs,
	Jaegeuk Kim

On 20.05.2022 09:30, Johannes Thumshirn wrote:
>On 20/05/2022 08:27, Javier González wrote:
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
>
>I personally don't like the padding we need to do for the super block.
>
>As I've pointed out to Pankaj already, I don't think it is 100% powerfail
>safe as of now. It could probably be made, but that would also involve
>changing non-zoned btrfs code which we try to avoid as much as we can.
>
>As Damien already said, we still have issues with the general zoned
>support in btrfs, just have a look at the list of open issues [1] we
>have.
>
Sounds good Johannes. I understand that the priority is to make btrfs
stable now, before introducing more variables. Let's stick to this and
then we can bring it back as the list of open issues becomes more
manageable.

>[1] https://protect2.fireeye.com/v1/url?k=f14a1d6f-90c10859-f14b9620-74fe485fffe0-3f1861e7739d8cc7&q=1&e=213fcc28-3f9d-41a1-b653-0dc0e203c718&u=https%3A%2F%2Fgithub.com%2Fnaota%2Flinux%2Fissues%2F

Thanks for sharing this too. It is a good way to where to help

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20 10:16                           ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20 10:16 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, dm-devel, pankydev8,
	gost.dev, Damien Le Moal, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, dsterba, Luis Chamberlain, linux-fsdevel,
	Jaegeuk Kim, Christoph Hellwig, linux-btrfs

On 20.05.2022 09:30, Johannes Thumshirn wrote:
>On 20/05/2022 08:27, Javier González wrote:
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
>
>I personally don't like the padding we need to do for the super block.
>
>As I've pointed out to Pankaj already, I don't think it is 100% powerfail
>safe as of now. It could probably be made, but that would also involve
>changing non-zoned btrfs code which we try to avoid as much as we can.
>
>As Damien already said, we still have issues with the general zoned
>support in btrfs, just have a look at the list of open issues [1] we
>have.
>
Sounds good Johannes. I understand that the priority is to make btrfs
stable now, before introducing more variables. Let's stick to this and
then we can bring it back as the list of open issues becomes more
manageable.

>[1] https://protect2.fireeye.com/v1/url?k=f14a1d6f-90c10859-f14b9620-74fe485fffe0-3f1861e7739d8cc7&q=1&e=213fcc28-3f9d-41a1-b653-0dc0e203c718&u=https%3A%2F%2Fgithub.com%2Fnaota%2Flinux%2Fissues%2F

Thanks for sharing this too. It is a good way to where to help

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  6:27                     ` Javier González
@ 2022-05-20  9:30                       ` Johannes Thumshirn
  -1 siblings, 0 replies; 56+ messages in thread
From: Johannes Thumshirn @ 2022-05-20  9:30 UTC (permalink / raw)
  To: Javier González, Hannes Reinecke
  Cc: Damien Le Moal, Luis Chamberlain, Theodore Ts'o,
	Christoph Hellwig, Pankaj Raghav, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim

On 20/05/2022 08:27, Javier González wrote:
> So you are suggesting adding support for !PO2 in the block layer and
> then a dm to present the device as a PO2 to the FS? This at least
> addresses the hole issue for raw zoned block devices, so it can be a
> first step.
> 
> This said, it seems to me that the changes to the FS are not being a
> real issue. In fact, we are exposing some bugs while we generalize the
> zone size support.
> 
> Could you point out what the challenges in btrfs are in the current
> patches, that it makes sense to add an extra dm layer?

I personally don't like the padding we need to do for the super block.

As I've pointed out to Pankaj already, I don't think it is 100% powerfail
safe as of now. It could probably be made, but that would also involve
changing non-zoned btrfs code which we try to avoid as much as we can.

As Damien already said, we still have issues with the general zoned 
support in btrfs, just have a look at the list of open issues [1] we
have. 

[1] https://github.com/naota/linux/issues/

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  9:30                       ` Johannes Thumshirn
  0 siblings, 0 replies; 56+ messages in thread
From: Johannes Thumshirn @ 2022-05-20  9:30 UTC (permalink / raw)
  To: Javier González, Hannes Reinecke
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, dm-devel, pankydev8,
	gost.dev, Damien Le Moal, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, dsterba, Luis Chamberlain, linux-fsdevel,
	Jaegeuk Kim, Christoph Hellwig, linux-btrfs

On 20/05/2022 08:27, Javier González wrote:
> So you are suggesting adding support for !PO2 in the block layer and
> then a dm to present the device as a PO2 to the FS? This at least
> addresses the hole issue for raw zoned block devices, so it can be a
> first step.
> 
> This said, it seems to me that the changes to the FS are not being a
> real issue. In fact, we are exposing some bugs while we generalize the
> zone size support.
> 
> Could you point out what the challenges in btrfs are in the current
> patches, that it makes sense to add an extra dm layer?

I personally don't like the padding we need to do for the super block.

As I've pointed out to Pankaj already, I don't think it is 100% powerfail
safe as of now. It could probably be made, but that would also involve
changing non-zoned btrfs code which we try to avoid as much as we can.

As Damien already said, we still have issues with the general zoned 
support in btrfs, just have a look at the list of open issues [1] we
have. 

[1] https://github.com/naota/linux/issues/


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  6:41                       ` Damien Le Moal
@ 2022-05-20  9:30                         ` Pankaj Raghav
  -1 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-20  9:30 UTC (permalink / raw)
  To: Damien Le Moal, Javier González, Hannes Reinecke,
	Johannes Thumshirn, Mike Snitzer, Christoph Hellwig
  Cc: Luis Chamberlain, Theodore Ts'o, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim,
	Keith Busch, Adam Manzanares

On 5/20/22 08:41, Damien Le Moal wrote:
>>>>>
>>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>>> dm-linear.
>>>>
Keith and Adam had a similar suggestion to go create a device mapper
(dm-unholy) when we tried the po2 emulation[1].
>>>> +1
>>>>
>>>> This will simplify the support for FSes, at least for the initial drop (if
>>>> accepted).
>>>>
>>>> And more importantly, this will also allow addressing any potential
>>>> problem with user space breaking because of the non power of 2 zone size.
>>>>
>>> Seconded (or maybe thirded).
>>>
>>> The changes to support npo2 in the block layer are pretty simple, and 
>>> really I don't have an issue with those.
>>> Then adding a device-mapper target transforming npo2 drives in po2 
>>> block devices should be pretty trivial.
>>>
>>> And once that is in you can start arguing with the the FS folks on 
>>> whether to implement it natively.
>>>
>>
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
> 
> Yes, and it also allows supporting these new !po2 devices without
> regressions (read lack of) in the support at FS level.
> 
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
> 
> Not arguing with that. But since we are still stabilizing btrfs ZNS
> support, adding more code right now is a little painful.
> 
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
> 
> See above. No real challenge, just needs to be done if a clear agreement
> can be reached on zone size alignment constraints. As mentioned above, the
> btrfs changes timing is not ideal right now though.
> 
> Also please do not forget applications that may expect a power of 2 zone
> size. A dm-zsp2 would be a nice solution for these. So regardless of the
> FS work, that new DM target will be *very* nice to have.
> 
>>
>> Note that for F2FS there is no blocker. Jaegeuk picked the initial
>> patches, and he agreed to add native support.
> 
> And until that is done, f2fs will not work with these new !po2 devices...
> Having the new dm will avoid that support fragmentation which I personally
> really dislike. With the new dm, we can keep support for *all* zoned block
> devices, albeit needing a different setup depending on the device. That is
> not nice at all but at least there is a way to make things work continuously.
> 

I see that many people in the community feel it is better to target the
dm layer for the initial support of npo2 devices. I can give it a shot
and maintain a native out-of-tree support for FSs for npo2 devices and
merge it upstream as we see fit later.

[1]
https://lore.kernel.org/all/20220311223032.GA2439@dhcp-10-100-145-180.wdc.com/

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  9:30                         ` Pankaj Raghav
  0 siblings, 0 replies; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-20  9:30 UTC (permalink / raw)
  To: Damien Le Moal, Javier González, Hannes Reinecke,
	Johannes Thumshirn, Mike Snitzer, Christoph Hellwig
  Cc: axboe, Theodore Ts'o, dm-devel, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	Luis Chamberlain, Adam Manzanares, linux-fsdevel, Jaegeuk Kim,
	Keith Busch, linux-btrfs

On 5/20/22 08:41, Damien Le Moal wrote:
>>>>>
>>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>>> dm-linear.
>>>>
Keith and Adam had a similar suggestion to go create a device mapper
(dm-unholy) when we tried the po2 emulation[1].
>>>> +1
>>>>
>>>> This will simplify the support for FSes, at least for the initial drop (if
>>>> accepted).
>>>>
>>>> And more importantly, this will also allow addressing any potential
>>>> problem with user space breaking because of the non power of 2 zone size.
>>>>
>>> Seconded (or maybe thirded).
>>>
>>> The changes to support npo2 in the block layer are pretty simple, and 
>>> really I don't have an issue with those.
>>> Then adding a device-mapper target transforming npo2 drives in po2 
>>> block devices should be pretty trivial.
>>>
>>> And once that is in you can start arguing with the the FS folks on 
>>> whether to implement it natively.
>>>
>>
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
> 
> Yes, and it also allows supporting these new !po2 devices without
> regressions (read lack of) in the support at FS level.
> 
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
> 
> Not arguing with that. But since we are still stabilizing btrfs ZNS
> support, adding more code right now is a little painful.
> 
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
> 
> See above. No real challenge, just needs to be done if a clear agreement
> can be reached on zone size alignment constraints. As mentioned above, the
> btrfs changes timing is not ideal right now though.
> 
> Also please do not forget applications that may expect a power of 2 zone
> size. A dm-zsp2 would be a nice solution for these. So regardless of the
> FS work, that new DM target will be *very* nice to have.
> 
>>
>> Note that for F2FS there is no blocker. Jaegeuk picked the initial
>> patches, and he agreed to add native support.
> 
> And until that is done, f2fs will not work with these new !po2 devices...
> Having the new dm will avoid that support fragmentation which I personally
> really dislike. With the new dm, we can keep support for *all* zoned block
> devices, albeit needing a different setup depending on the device. That is
> not nice at all but at least there is a way to make things work continuously.
> 

I see that many people in the community feel it is better to target the
dm layer for the initial support of npo2 devices. I can give it a shot
and maintain a native out-of-tree support for FSs for npo2 devices and
merge it upstream as we see fit later.

[1]
https://lore.kernel.org/all/20220311223032.GA2439@dhcp-10-100-145-180.wdc.com/

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
       [not found]                       ` <CGME20220520065941eucas1p105cf273ede995dc4bf92f3245fad09b1@eucas1p1.samsung.com>
@ 2022-05-20  6:59                           ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20  6:59 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Hannes Reinecke, Johannes Thumshirn, Luis Chamberlain,
	Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs,
	Jaegeuk Kim

On 20.05.2022 15:41, Damien Le Moal wrote:
>On 5/20/22 15:27, Javier González wrote:
>> On 20.05.2022 08:07, Hannes Reinecke wrote:
>>> On 5/19/22 20:47, Damien Le Moal wrote:
>>>> On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>>> I'm a little surprised about all this activity.
>>>>>>>>>>
>>>>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>>>>
>>>>>>>>> FWIW,
>>>>>>>>>
>>>>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>>>>> videos become available, folks can decide for themselves.
>>>>>>>>
>>>>>>>> There was no real discussion about zone size constraint on the zone
>>>>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>>>>
>>>>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>>>>
>>>>>> Nor any clear OK.
>>>>>
>>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>>> dm-linear.
>>>>
>>>> +1
>>>>
>>>> This will simplify the support for FSes, at least for the initial drop (if
>>>> accepted).
>>>>
>>>> And more importantly, this will also allow addressing any potential
>>>> problem with user space breaking because of the non power of 2 zone size.
>>>>
>>> Seconded (or maybe thirded).
>>>
>>> The changes to support npo2 in the block layer are pretty simple, and
>>> really I don't have an issue with those.
>>> Then adding a device-mapper target transforming npo2 drives in po2
>>> block devices should be pretty trivial.
>>>
>>> And once that is in you can start arguing with the the FS folks on
>>> whether to implement it natively.
>>>
>>
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
>
>Yes, and it also allows supporting these new !po2 devices without
>regressions (read lack of) in the support at FS level.
>
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
>
>Not arguing with that. But since we are still stabilizing btrfs ZNS
>support, adding more code right now is a little painful.
>
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
>
>See above. No real challenge, just needs to be done if a clear agreement
>can be reached on zone size alignment constraints. As mentioned above, the
>btrfs changes timing is not ideal right now though.
>
>Also please do not forget applications that may expect a power of 2 zone
>size. A dm-zsp2 would be a nice solution for these. So regardless of the
>FS work, that new DM target will be *very* nice to have.
>
>>
>> Note that for F2FS there is no blocker. Jaegeuk picked the initial
>> patches, and he agreed to add native support.
>
>And until that is done, f2fs will not work with these new !po2 devices...
>Having the new dm will avoid that support fragmentation which I personally
>really dislike. With the new dm, we can keep support for *all* zoned block
>devices, albeit needing a different setup depending on the device. That is
>not nice at all but at least there is a way to make things work continuously.

All the above sounds very reasonable. Thanks Damien.

If we all can agree, we can address this in the next version and come
maintain the native FS support off-tree until you see that general btrfs
support for zoned devicse is stable. We will be happy to help with this
too.


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  6:59                           ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20  6:59 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, dm-devel, pankydev8,
	Johannes Thumshirn, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, dsterba, Luis Chamberlain, gost.dev, linux-fsdevel,
	Jaegeuk Kim, Christoph Hellwig, linux-btrfs

On 20.05.2022 15:41, Damien Le Moal wrote:
>On 5/20/22 15:27, Javier González wrote:
>> On 20.05.2022 08:07, Hannes Reinecke wrote:
>>> On 5/19/22 20:47, Damien Le Moal wrote:
>>>> On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>>> I'm a little surprised about all this activity.
>>>>>>>>>>
>>>>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>>>>
>>>>>>>>> FWIW,
>>>>>>>>>
>>>>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>>>>> videos become available, folks can decide for themselves.
>>>>>>>>
>>>>>>>> There was no real discussion about zone size constraint on the zone
>>>>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>>>>
>>>>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>>>>
>>>>>> Nor any clear OK.
>>>>>
>>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>>> dm-linear.
>>>>
>>>> +1
>>>>
>>>> This will simplify the support for FSes, at least for the initial drop (if
>>>> accepted).
>>>>
>>>> And more importantly, this will also allow addressing any potential
>>>> problem with user space breaking because of the non power of 2 zone size.
>>>>
>>> Seconded (or maybe thirded).
>>>
>>> The changes to support npo2 in the block layer are pretty simple, and
>>> really I don't have an issue with those.
>>> Then adding a device-mapper target transforming npo2 drives in po2
>>> block devices should be pretty trivial.
>>>
>>> And once that is in you can start arguing with the the FS folks on
>>> whether to implement it natively.
>>>
>>
>> So you are suggesting adding support for !PO2 in the block layer and
>> then a dm to present the device as a PO2 to the FS? This at least
>> addresses the hole issue for raw zoned block devices, so it can be a
>> first step.
>
>Yes, and it also allows supporting these new !po2 devices without
>regressions (read lack of) in the support at FS level.
>
>>
>> This said, it seems to me that the changes to the FS are not being a
>> real issue. In fact, we are exposing some bugs while we generalize the
>> zone size support.
>
>Not arguing with that. But since we are still stabilizing btrfs ZNS
>support, adding more code right now is a little painful.
>
>>
>> Could you point out what the challenges in btrfs are in the current
>> patches, that it makes sense to add an extra dm layer?
>
>See above. No real challenge, just needs to be done if a clear agreement
>can be reached on zone size alignment constraints. As mentioned above, the
>btrfs changes timing is not ideal right now though.
>
>Also please do not forget applications that may expect a power of 2 zone
>size. A dm-zsp2 would be a nice solution for these. So regardless of the
>FS work, that new DM target will be *very* nice to have.
>
>>
>> Note that for F2FS there is no blocker. Jaegeuk picked the initial
>> patches, and he agreed to add native support.
>
>And until that is done, f2fs will not work with these new !po2 devices...
>Having the new dm will avoid that support fragmentation which I personally
>really dislike. With the new dm, we can keep support for *all* zoned block
>devices, albeit needing a different setup depending on the device. That is
>not nice at all but at least there is a way to make things work continuously.

All the above sounds very reasonable. Thanks Damien.

If we all can agree, we can address this in the next version and come
maintain the native FS support off-tree until you see that general btrfs
support for zoned devicse is stable. We will be happy to help with this
too.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  6:27                     ` Javier González
@ 2022-05-20  6:41                       ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-20  6:41 UTC (permalink / raw)
  To: Javier González, Hannes Reinecke
  Cc: Johannes Thumshirn, Luis Chamberlain, Theodore Ts'o,
	Christoph Hellwig, Pankaj Raghav, axboe, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs, Jaegeuk Kim

On 5/20/22 15:27, Javier González wrote:
> On 20.05.2022 08:07, Hannes Reinecke wrote:
>> On 5/19/22 20:47, Damien Le Moal wrote:
>>> On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>> I'm a little surprised about all this activity.
>>>>>>>>>
>>>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>>>
>>>>>>>> FWIW,
>>>>>>>>
>>>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>>>> videos become available, folks can decide for themselves.
>>>>>>>
>>>>>>> There was no real discussion about zone size constraint on the zone
>>>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>>>
>>>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>>>
>>>>> Nor any clear OK.
>>>>
>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>> dm-linear.
>>>
>>> +1
>>>
>>> This will simplify the support for FSes, at least for the initial drop (if
>>> accepted).
>>>
>>> And more importantly, this will also allow addressing any potential
>>> problem with user space breaking because of the non power of 2 zone size.
>>>
>> Seconded (or maybe thirded).
>>
>> The changes to support npo2 in the block layer are pretty simple, and 
>> really I don't have an issue with those.
>> Then adding a device-mapper target transforming npo2 drives in po2 
>> block devices should be pretty trivial.
>>
>> And once that is in you can start arguing with the the FS folks on 
>> whether to implement it natively.
>>
> 
> So you are suggesting adding support for !PO2 in the block layer and
> then a dm to present the device as a PO2 to the FS? This at least
> addresses the hole issue for raw zoned block devices, so it can be a
> first step.

Yes, and it also allows supporting these new !po2 devices without
regressions (read lack of) in the support at FS level.

> 
> This said, it seems to me that the changes to the FS are not being a
> real issue. In fact, we are exposing some bugs while we generalize the
> zone size support.

Not arguing with that. But since we are still stabilizing btrfs ZNS
support, adding more code right now is a little painful.

> 
> Could you point out what the challenges in btrfs are in the current
> patches, that it makes sense to add an extra dm layer?

See above. No real challenge, just needs to be done if a clear agreement
can be reached on zone size alignment constraints. As mentioned above, the
btrfs changes timing is not ideal right now though.

Also please do not forget applications that may expect a power of 2 zone
size. A dm-zsp2 would be a nice solution for these. So regardless of the
FS work, that new DM target will be *very* nice to have.

> 
> Note that for F2FS there is no blocker. Jaegeuk picked the initial
> patches, and he agreed to add native support.

And until that is done, f2fs will not work with these new !po2 devices...
Having the new dm will avoid that support fragmentation which I personally
really dislike. With the new dm, we can keep support for *all* zoned block
devices, albeit needing a different setup depending on the device. That is
not nice at all but at least there is a way to make things work continuously.

-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  6:41                       ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-20  6:41 UTC (permalink / raw)
  To: Javier González, Hannes Reinecke
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, dm-devel, pankydev8,
	Johannes Thumshirn, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, dsterba, Luis Chamberlain, gost.dev, linux-fsdevel,
	Jaegeuk Kim, Christoph Hellwig, linux-btrfs

On 5/20/22 15:27, Javier González wrote:
> On 20.05.2022 08:07, Hannes Reinecke wrote:
>> On 5/19/22 20:47, Damien Le Moal wrote:
>>> On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>> I'm a little surprised about all this activity.
>>>>>>>>>
>>>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>>>
>>>>>>>> FWIW,
>>>>>>>>
>>>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>>>> videos become available, folks can decide for themselves.
>>>>>>>
>>>>>>> There was no real discussion about zone size constraint on the zone
>>>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>>>
>>>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>>>
>>>>> Nor any clear OK.
>>>>
>>>> So what about creating a device-mapper target, that's taking npo2 drives and
>>>> makes them po2 drives for the FS layers? It will be very similar code to
>>>> dm-linear.
>>>
>>> +1
>>>
>>> This will simplify the support for FSes, at least for the initial drop (if
>>> accepted).
>>>
>>> And more importantly, this will also allow addressing any potential
>>> problem with user space breaking because of the non power of 2 zone size.
>>>
>> Seconded (or maybe thirded).
>>
>> The changes to support npo2 in the block layer are pretty simple, and 
>> really I don't have an issue with those.
>> Then adding a device-mapper target transforming npo2 drives in po2 
>> block devices should be pretty trivial.
>>
>> And once that is in you can start arguing with the the FS folks on 
>> whether to implement it natively.
>>
> 
> So you are suggesting adding support for !PO2 in the block layer and
> then a dm to present the device as a PO2 to the FS? This at least
> addresses the hole issue for raw zoned block devices, so it can be a
> first step.

Yes, and it also allows supporting these new !po2 devices without
regressions (read lack of) in the support at FS level.

> 
> This said, it seems to me that the changes to the FS are not being a
> real issue. In fact, we are exposing some bugs while we generalize the
> zone size support.

Not arguing with that. But since we are still stabilizing btrfs ZNS
support, adding more code right now is a little painful.

> 
> Could you point out what the challenges in btrfs are in the current
> patches, that it makes sense to add an extra dm layer?

See above. No real challenge, just needs to be done if a clear agreement
can be reached on zone size alignment constraints. As mentioned above, the
btrfs changes timing is not ideal right now though.

Also please do not forget applications that may expect a power of 2 zone
size. A dm-zsp2 would be a nice solution for these. So regardless of the
FS work, that new DM target will be *very* nice to have.

> 
> Note that for F2FS there is no blocker. Jaegeuk picked the initial
> patches, and he agreed to add native support.

And until that is done, f2fs will not work with these new !po2 devices...
Having the new dm will avoid that support fragmentation which I personally
really dislike. With the new dm, we can keep support for *all* zoned block
devices, albeit needing a different setup depending on the device. That is
not nice at all but at least there is a way to make things work continuously.

-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  6:07                   ` Hannes Reinecke
@ 2022-05-20  6:27                     ` Javier González
  -1 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20  6:27 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Damien Le Moal, Johannes Thumshirn, Luis Chamberlain,
	Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs,
	Jaegeuk Kim

On 20.05.2022 08:07, Hannes Reinecke wrote:
>On 5/19/22 20:47, Damien Le Moal wrote:
>>On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>I'm a little surprised about all this activity.
>>>>>>>>
>>>>>>>>I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>is very little benefit in supporting this scheme.  It will massively
>>>>>>>>fragment the supported based of devices and applications, while only
>>>>>>>>having the benefit of supporting some Samsung legacy devices.
>>>>>>>
>>>>>>>FWIW,
>>>>>>>
>>>>>>>That wasn't my impression from that LSF/MM session, but once the
>>>>>>>videos become available, folks can decide for themselves.
>>>>>>
>>>>>>There was no real discussion about zone size constraint on the zone
>>>>>>storage BoF. Many discussions happened in the hallway track though.
>>>>>
>>>>>Right so no direct clear blockers mentioned at all during the BoF.
>>>>
>>>>Nor any clear OK.
>>>
>>>So what about creating a device-mapper target, that's taking npo2 drives and
>>>makes them po2 drives for the FS layers? It will be very similar code to
>>>dm-linear.
>>
>>+1
>>
>>This will simplify the support for FSes, at least for the initial drop (if
>>accepted).
>>
>>And more importantly, this will also allow addressing any potential
>>problem with user space breaking because of the non power of 2 zone size.
>>
>Seconded (or maybe thirded).
>
>The changes to support npo2 in the block layer are pretty simple, and 
>really I don't have an issue with those.
>Then adding a device-mapper target transforming npo2 drives in po2 
>block devices should be pretty trivial.
>
>And once that is in you can start arguing with the the FS folks on 
>whether to implement it natively.
>

So you are suggesting adding support for !PO2 in the block layer and
then a dm to present the device as a PO2 to the FS? This at least
addresses the hole issue for raw zoned block devices, so it can be a
first step.

This said, it seems to me that the changes to the FS are not being a
real issue. In fact, we are exposing some bugs while we generalize the
zone size support.

Could you point out what the challenges in btrfs are in the current
patches, that it makes sense to add an extra dm layer?

Note that for F2FS there is no blocker. Jaegeuk picked the initial
patches, and he agreed to add native support.

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  6:27                     ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-20  6:27 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, dm-devel, pankydev8,
	Johannes Thumshirn, Damien Le Moal, jiangbo.365, linux-nvme,
	linux-kernel, linux-block, dsterba, Luis Chamberlain, gost.dev,
	linux-fsdevel, Jaegeuk Kim, Christoph Hellwig, linux-btrfs

On 20.05.2022 08:07, Hannes Reinecke wrote:
>On 5/19/22 20:47, Damien Le Moal wrote:
>>On 5/19/22 16:34, Johannes Thumshirn wrote:
>>>On 19/05/2022 05:19, Damien Le Moal wrote:
>>>>On 5/19/22 12:12, Luis Chamberlain wrote:
>>>>>On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>>>On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>>>On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>>>I'm a little surprised about all this activity.
>>>>>>>>
>>>>>>>>I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>>>is very little benefit in supporting this scheme.  It will massively
>>>>>>>>fragment the supported based of devices and applications, while only
>>>>>>>>having the benefit of supporting some Samsung legacy devices.
>>>>>>>
>>>>>>>FWIW,
>>>>>>>
>>>>>>>That wasn't my impression from that LSF/MM session, but once the
>>>>>>>videos become available, folks can decide for themselves.
>>>>>>
>>>>>>There was no real discussion about zone size constraint on the zone
>>>>>>storage BoF. Many discussions happened in the hallway track though.
>>>>>
>>>>>Right so no direct clear blockers mentioned at all during the BoF.
>>>>
>>>>Nor any clear OK.
>>>
>>>So what about creating a device-mapper target, that's taking npo2 drives and
>>>makes them po2 drives for the FS layers? It will be very similar code to
>>>dm-linear.
>>
>>+1
>>
>>This will simplify the support for FSes, at least for the initial drop (if
>>accepted).
>>
>>And more importantly, this will also allow addressing any potential
>>problem with user space breaking because of the non power of 2 zone size.
>>
>Seconded (or maybe thirded).
>
>The changes to support npo2 in the block layer are pretty simple, and 
>really I don't have an issue with those.
>Then adding a device-mapper target transforming npo2 drives in po2 
>block devices should be pretty trivial.
>
>And once that is in you can start arguing with the the FS folks on 
>whether to implement it natively.
>

So you are suggesting adding support for !PO2 in the block layer and
then a dm to present the device as a PO2 to the FS? This at least
addresses the hole issue for raw zoned block devices, so it can be a
first step.

This said, it seems to me that the changes to the FS are not being a
real issue. In fact, we are exposing some bugs while we generalize the
zone size support.

Could you point out what the challenges in btrfs are in the current
patches, that it makes sense to add an extra dm layer?

Note that for F2FS there is no blocker. Jaegeuk picked the initial
patches, and he agreed to add native support.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-20  3:47                 ` Damien Le Moal
@ 2022-05-20  6:07                   ` Hannes Reinecke
  -1 siblings, 0 replies; 56+ messages in thread
From: Hannes Reinecke @ 2022-05-20  6:07 UTC (permalink / raw)
  To: Damien Le Moal, Johannes Thumshirn, Luis Chamberlain
  Cc: Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On 5/19/22 20:47, Damien Le Moal wrote:
> On 5/19/22 16:34, Johannes Thumshirn wrote:
>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>> I'm a little surprised about all this activity.
>>>>>>>
>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>
>>>>>> FWIW,
>>>>>>
>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>> videos become available, folks can decide for themselves.
>>>>>
>>>>> There was no real discussion about zone size constraint on the zone
>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>
>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>
>>> Nor any clear OK.
>>
>> So what about creating a device-mapper target, that's taking npo2 drives and
>> makes them po2 drives for the FS layers? It will be very similar code to
>> dm-linear.
> 
> +1
> 
> This will simplify the support for FSes, at least for the initial drop (if
> accepted).
> 
> And more importantly, this will also allow addressing any potential
> problem with user space breaking because of the non power of 2 zone size.
> 
Seconded (or maybe thirded).

The changes to support npo2 in the block layer are pretty simple, and 
really I don't have an issue with those.
Then adding a device-mapper target transforming npo2 drives in po2 block 
devices should be pretty trivial.

And once that is in you can start arguing with the the FS folks on 
whether to implement it natively.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  6:07                   ` Hannes Reinecke
  0 siblings, 0 replies; 56+ messages in thread
From: Hannes Reinecke @ 2022-05-20  6:07 UTC (permalink / raw)
  To: Damien Le Moal, Johannes Thumshirn, Luis Chamberlain
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On 5/19/22 20:47, Damien Le Moal wrote:
> On 5/19/22 16:34, Johannes Thumshirn wrote:
>> On 19/05/2022 05:19, Damien Le Moal wrote:
>>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>>> I'm a little surprised about all this activity.
>>>>>>>
>>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>>> fragment the supported based of devices and applications, while only
>>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>>
>>>>>> FWIW,
>>>>>>
>>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>>> videos become available, folks can decide for themselves.
>>>>>
>>>>> There was no real discussion about zone size constraint on the zone
>>>>> storage BoF. Many discussions happened in the hallway track though.
>>>>
>>>> Right so no direct clear blockers mentioned at all during the BoF.
>>>
>>> Nor any clear OK.
>>
>> So what about creating a device-mapper target, that's taking npo2 drives and
>> makes them po2 drives for the FS layers? It will be very similar code to
>> dm-linear.
> 
> +1
> 
> This will simplify the support for FSes, at least for the initial drop (if
> accepted).
> 
> And more importantly, this will also allow addressing any potential
> problem with user space breaking because of the non power of 2 zone size.
> 
Seconded (or maybe thirded).

The changes to support npo2 in the block layer are pretty simple, and 
really I don't have an issue with those.
Then adding a device-mapper target transforming npo2 drives in po2 block 
devices should be pretty trivial.

And once that is in you can start arguing with the the FS folks on 
whether to implement it natively.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-19  7:34               ` Johannes Thumshirn
@ 2022-05-20  3:47                 ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-20  3:47 UTC (permalink / raw)
  To: Johannes Thumshirn, Luis Chamberlain
  Cc: Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On 5/19/22 16:34, Johannes Thumshirn wrote:
> On 19/05/2022 05:19, Damien Le Moal wrote:
>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>> I'm a little surprised about all this activity.
>>>>>>
>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>> fragment the supported based of devices and applications, while only
>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>
>>>>> FWIW,
>>>>>
>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>> videos become available, folks can decide for themselves.
>>>>
>>>> There was no real discussion about zone size constraint on the zone
>>>> storage BoF. Many discussions happened in the hallway track though.
>>>
>>> Right so no direct clear blockers mentioned at all during the BoF.
>>
>> Nor any clear OK.
> 
> So what about creating a device-mapper target, that's taking npo2 drives and
> makes them po2 drives for the FS layers? It will be very similar code to 
> dm-linear.

+1

This will simplify the support for FSes, at least for the initial drop (if
accepted).

And more importantly, this will also allow addressing any potential
problem with user space breaking because of the non power of 2 zone size.

> 
> After all zoned support for FSes started with a device-mapper (dm-zoned) and 
> as the need for a more integrated solution arose, it changed into natiive
> support.
> 
> And all that is there is simple arithmetic and a bio_clone(), if this is the
> slowest part of the stack involving a FS like f2fs or btrfs I'm throwing a
> round of anyone's favorite beverage at next year's LSFMM.
> 
> Byte,
> 	Johannes
> 


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-20  3:47                 ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-20  3:47 UTC (permalink / raw)
  To: Johannes Thumshirn, Luis Chamberlain
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On 5/19/22 16:34, Johannes Thumshirn wrote:
> On 19/05/2022 05:19, Damien Le Moal wrote:
>> On 5/19/22 12:12, Luis Chamberlain wrote:
>>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>>> I'm a little surprised about all this activity.
>>>>>>
>>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>>> fragment the supported based of devices and applications, while only
>>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>>
>>>>> FWIW,
>>>>>
>>>>> That wasn't my impression from that LSF/MM session, but once the
>>>>> videos become available, folks can decide for themselves.
>>>>
>>>> There was no real discussion about zone size constraint on the zone
>>>> storage BoF. Many discussions happened in the hallway track though.
>>>
>>> Right so no direct clear blockers mentioned at all during the BoF.
>>
>> Nor any clear OK.
> 
> So what about creating a device-mapper target, that's taking npo2 drives and
> makes them po2 drives for the FS layers? It will be very similar code to 
> dm-linear.

+1

This will simplify the support for FSes, at least for the initial drop (if
accepted).

And more importantly, this will also allow addressing any potential
problem with user space breaking because of the non power of 2 zone size.

> 
> After all zoned support for FSes started with a device-mapper (dm-zoned) and 
> as the need for a more integrated solution arose, it changed into natiive
> support.
> 
> And all that is there is simple arithmetic and a bio_clone(), if this is the
> slowest part of the stack involving a FS like f2fs or btrfs I'm throwing a
> round of anyone's favorite beverage at next year's LSFMM.
> 
> Byte,
> 	Johannes
> 


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-18  8:00       ` Christoph Hellwig
@ 2022-05-19 15:25         ` Javier González
  0 siblings, 0 replies; 56+ messages in thread
From: Javier González @ 2022-05-19 15:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, Javier González,
	linux-btrfs


> On 18 May 2022, at 10.16, Christoph Hellwig <hch@lst.de> wrote:
> 
> On Tue, May 17, 2022 at 11:18:34AM +0200, Javier González wrote:
>> Does the above help you reconsidering your interest in supporting this
>> in NVMe?
> 
> Very little.  It just seems like a really bad idea.

I understand you don’t like this, but I still hope you see value in supporting it. We are getting close to a very minimal patchset, which is also helping to fix bugs in the zoned stack.

If you take a look at the last version abs give some feedback, I’m sure we can end up with a good solution. 

Can you help?

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-19  3:19             ` Damien Le Moal
@ 2022-05-19  7:34               ` Johannes Thumshirn
  -1 siblings, 0 replies; 56+ messages in thread
From: Johannes Thumshirn @ 2022-05-19  7:34 UTC (permalink / raw)
  To: Damien Le Moal, Luis Chamberlain
  Cc: Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On 19/05/2022 05:19, Damien Le Moal wrote:
> On 5/19/22 12:12, Luis Chamberlain wrote:
>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>> I'm a little surprised about all this activity.
>>>>>
>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>> fragment the supported based of devices and applications, while only
>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>
>>>> FWIW,
>>>>
>>>> That wasn't my impression from that LSF/MM session, but once the
>>>> videos become available, folks can decide for themselves.
>>>
>>> There was no real discussion about zone size constraint on the zone
>>> storage BoF. Many discussions happened in the hallway track though.
>>
>> Right so no direct clear blockers mentioned at all during the BoF.
> 
> Nor any clear OK.

So what about creating a device-mapper target, that's taking npo2 drives and
makes them po2 drives for the FS layers? It will be very similar code to 
dm-linear.

After all zoned support for FSes started with a device-mapper (dm-zoned) and 
as the need for a more integrated solution arose, it changed into natiive
support.

And all that is there is simple arithmetic and a bio_clone(), if this is the
slowest part of the stack involving a FS like f2fs or btrfs I'm throwing a
round of anyone's favorite beverage at next year's LSFMM.

Byte,
	Johannes


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-19  7:34               ` Johannes Thumshirn
  0 siblings, 0 replies; 56+ messages in thread
From: Johannes Thumshirn @ 2022-05-19  7:34 UTC (permalink / raw)
  To: Damien Le Moal, Luis Chamberlain
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On 19/05/2022 05:19, Damien Le Moal wrote:
> On 5/19/22 12:12, Luis Chamberlain wrote:
>> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>>> I'm a little surprised about all this activity.
>>>>>
>>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>>> is very little benefit in supporting this scheme.  It will massively
>>>>> fragment the supported based of devices and applications, while only
>>>>> having the benefit of supporting some Samsung legacy devices.
>>>>
>>>> FWIW,
>>>>
>>>> That wasn't my impression from that LSF/MM session, but once the
>>>> videos become available, folks can decide for themselves.
>>>
>>> There was no real discussion about zone size constraint on the zone
>>> storage BoF. Many discussions happened in the hallway track though.
>>
>> Right so no direct clear blockers mentioned at all during the BoF.
> 
> Nor any clear OK.

So what about creating a device-mapper target, that's taking npo2 drives and
makes them po2 drives for the FS layers? It will be very similar code to 
dm-linear.

After all zoned support for FSes started with a device-mapper (dm-zoned) and 
as the need for a more integrated solution arose, it changed into natiive
support.

And all that is there is simple arithmetic and a bio_clone(), if this is the
slowest part of the stack involving a FS like f2fs or btrfs I'm throwing a
round of anyone's favorite beverage at next year's LSFMM.

Byte,
	Johannes



--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-19  3:12           ` Luis Chamberlain
@ 2022-05-19  3:19             ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-19  3:19 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On 5/19/22 12:12, Luis Chamberlain wrote:
> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>> I'm a little surprised about all this activity.
>>>>
>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>> is very little benefit in supporting this scheme.  It will massively
>>>> fragment the supported based of devices and applications, while only
>>>> having the benefit of supporting some Samsung legacy devices.
>>>
>>> FWIW,
>>>
>>> That wasn't my impression from that LSF/MM session, but once the
>>> videos become available, folks can decide for themselves.
>>
>> There was no real discussion about zone size constraint on the zone
>> storage BoF. Many discussions happened in the hallway track though.
> 
> Right so no direct clear blockers mentioned at all during the BoF.

Nor any clear OK.

> 
>   Luis


-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-19  3:19             ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-19  3:19 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On 5/19/22 12:12, Luis Chamberlain wrote:
> On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
>> On 5/18/22 00:34, Theodore Ts'o wrote:
>>> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>>>> I'm a little surprised about all this activity.
>>>>
>>>> I though the conclusion at LSF/MM was that for Linux itself there
>>>> is very little benefit in supporting this scheme.  It will massively
>>>> fragment the supported based of devices and applications, while only
>>>> having the benefit of supporting some Samsung legacy devices.
>>>
>>> FWIW,
>>>
>>> That wasn't my impression from that LSF/MM session, but once the
>>> videos become available, folks can decide for themselves.
>>
>> There was no real discussion about zone size constraint on the zone
>> storage BoF. Many discussions happened in the hallway track though.
> 
> Right so no direct clear blockers mentioned at all during the BoF.

Nor any clear OK.

> 
>   Luis


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-19  3:08         ` Damien Le Moal
@ 2022-05-19  3:12           ` Luis Chamberlain
  -1 siblings, 0 replies; 56+ messages in thread
From: Luis Chamberlain @ 2022-05-19  3:12 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Theodore Ts'o, Christoph Hellwig, Pankaj Raghav, axboe,
	pankydev8, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
> On 5/18/22 00:34, Theodore Ts'o wrote:
> > On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> >> I'm a little surprised about all this activity.
> >>
> >> I though the conclusion at LSF/MM was that for Linux itself there
> >> is very little benefit in supporting this scheme.  It will massively
> >> fragment the supported based of devices and applications, while only
> >> having the benefit of supporting some Samsung legacy devices.
> > 
> > FWIW,
> > 
> > That wasn't my impression from that LSF/MM session, but once the
> > videos become available, folks can decide for themselves.
> 
> There was no real discussion about zone size constraint on the zone
> storage BoF. Many discussions happened in the hallway track though.

Right so no direct clear blockers mentioned at all during the BoF.

  Luis

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-19  3:12           ` Luis Chamberlain
  0 siblings, 0 replies; 56+ messages in thread
From: Luis Chamberlain @ 2022-05-19  3:12 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Pankaj Raghav, axboe, Theodore Ts'o, pankydev8, gost.dev,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On Thu, May 19, 2022 at 12:08:26PM +0900, Damien Le Moal wrote:
> On 5/18/22 00:34, Theodore Ts'o wrote:
> > On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> >> I'm a little surprised about all this activity.
> >>
> >> I though the conclusion at LSF/MM was that for Linux itself there
> >> is very little benefit in supporting this scheme.  It will massively
> >> fragment the supported based of devices and applications, while only
> >> having the benefit of supporting some Samsung legacy devices.
> > 
> > FWIW,
> > 
> > That wasn't my impression from that LSF/MM session, but once the
> > videos become available, folks can decide for themselves.
> 
> There was no real discussion about zone size constraint on the zone
> storage BoF. Many discussions happened in the hallway track though.

Right so no direct clear blockers mentioned at all during the BoF.

  Luis

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-17 15:34       ` Theodore Ts'o
@ 2022-05-19  3:08         ` Damien Le Moal
  -1 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-19  3:08 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, jiangbo.365,
	linux-nvme, linux-kernel, linux-block, linux-fsdevel, dm-devel,
	dsterba, linux-btrfs

On 5/18/22 00:34, Theodore Ts'o wrote:
> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>> I'm a little surprised about all this activity.
>>
>> I though the conclusion at LSF/MM was that for Linux itself there
>> is very little benefit in supporting this scheme.  It will massively
>> fragment the supported based of devices and applications, while only
>> having the benefit of supporting some Samsung legacy devices.
> 
> FWIW,
> 
> That wasn't my impression from that LSF/MM session, but once the
> videos become available, folks can decide for themselves.

There was no real discussion about zone size constraint on the zone
storage BoF. Many discussions happened in the hallway track though.

-- 
Damien Le Moal
Western Digital Research

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-19  3:08         ` Damien Le Moal
  0 siblings, 0 replies; 56+ messages in thread
From: Damien Le Moal @ 2022-05-19  3:08 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, jiangbo.365,
	linux-nvme, linux-kernel, linux-block, dsterba, dm-devel,
	linux-fsdevel, linux-btrfs

On 5/18/22 00:34, Theodore Ts'o wrote:
> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
>> I'm a little surprised about all this activity.
>>
>> I though the conclusion at LSF/MM was that for Linux itself there
>> is very little benefit in supporting this scheme.  It will massively
>> fragment the supported based of devices and applications, while only
>> having the benefit of supporting some Samsung legacy devices.
> 
> FWIW,
> 
> That wasn't my impression from that LSF/MM session, but once the
> videos become available, folks can decide for themselves.

There was no real discussion about zone size constraint on the zone
storage BoF. Many discussions happened in the hallway track though.

-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-17 15:34       ` Theodore Ts'o
@ 2022-05-18 23:06         ` Luis Chamberlain
  -1 siblings, 0 replies; 56+ messages in thread
From: Luis Chamberlain @ 2022-05-18 23:06 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Christoph Hellwig, Pankaj Raghav, axboe, pankydev8, gost.dev,
	damien.lemoal, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, linux-fsdevel, dm-devel, dsterba, linux-btrfs

On Tue, May 17, 2022 at 11:34:54AM -0400, Theodore Ts'o wrote:
> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> > I'm a little surprised about all this activity.
> > 
> > I though the conclusion at LSF/MM was that for Linux itself there
> > is very little benefit in supporting this scheme.  It will massively
> > fragment the supported based of devices and applications, while only
> > having the benefit of supporting some Samsung legacy devices.
> 
> FWIW,
> 
> That wasn't my impression from that LSF/MM session, but once the
> videos become available, folks can decide for themselves.

Agreed, contrary to conventional storage devices, with the zone storage
ecosystem we simply have a requirement of zone drive replacements matching
zone size. That requirement exists for po2 or npo2. The work in this patch
set proves that supporting npo2 was in the end straight forward. As the one
putting together the BoF I can say that there were no sticking points raised
to move forward with this when the topic came up. So I am very surprised to
hear about any other perceived conclusion.

  Luis

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-18 23:06         ` Luis Chamberlain
  0 siblings, 0 replies; 56+ messages in thread
From: Luis Chamberlain @ 2022-05-18 23:06 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, Christoph Hellwig, linux-btrfs

On Tue, May 17, 2022 at 11:34:54AM -0400, Theodore Ts'o wrote:
> On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> > I'm a little surprised about all this activity.
> > 
> > I though the conclusion at LSF/MM was that for Linux itself there
> > is very little benefit in supporting this scheme.  It will massively
> > fragment the supported based of devices and applications, while only
> > having the benefit of supporting some Samsung legacy devices.
> 
> FWIW,
> 
> That wasn't my impression from that LSF/MM session, but once the
> videos become available, folks can decide for themselves.

Agreed, contrary to conventional storage devices, with the zone storage
ecosystem we simply have a requirement of zone drive replacements matching
zone size. That requirement exists for po2 or npo2. The work in this patch
set proves that supporting npo2 was in the end straight forward. As the one
putting together the BoF I can say that there were no sticking points raised
to move forward with this when the topic came up. So I am very surprised to
hear about any other perceived conclusion.

  Luis

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-17  9:18     ` Javier González
@ 2022-05-18  8:00       ` Christoph Hellwig
  2022-05-19 15:25         ` Javier González
  0 siblings, 1 reply; 56+ messages in thread
From: Christoph Hellwig @ 2022-05-18  8:00 UTC (permalink / raw)
  To: Javier González
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, Christoph Hellwig, linux-btrfs

On Tue, May 17, 2022 at 11:18:34AM +0200, Javier González wrote:
> Does the above help you reconsidering your interest in supporting this
> in NVMe?

Very little.  It just seems like a really bad idea.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-17  8:10   ` Christoph Hellwig
@ 2022-05-17 15:34       ` Theodore Ts'o
  2022-05-17 15:34       ` Theodore Ts'o
  1 sibling, 0 replies; 56+ messages in thread
From: Theodore Ts'o @ 2022-05-17 15:34 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs

On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> I'm a little surprised about all this activity.
> 
> I though the conclusion at LSF/MM was that for Linux itself there
> is very little benefit in supporting this scheme.  It will massively
> fragment the supported based of devices and applications, while only
> having the benefit of supporting some Samsung legacy devices.

FWIW,

That wasn't my impression from that LSF/MM session, but once the
videos become available, folks can decide for themselves.

       	      		       	   	  - Ted

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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
@ 2022-05-17 15:34       ` Theodore Ts'o
  0 siblings, 0 replies; 56+ messages in thread
From: Theodore Ts'o @ 2022-05-17 15:34 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block, dsterba,
	dm-devel, linux-fsdevel, linux-btrfs

On Tue, May 17, 2022 at 10:10:48AM +0200, Christoph Hellwig wrote:
> I'm a little surprised about all this activity.
> 
> I though the conclusion at LSF/MM was that for Linux itself there
> is very little benefit in supporting this scheme.  It will massively
> fragment the supported based of devices and applications, while only
> having the benefit of supporting some Samsung legacy devices.

FWIW,

That wasn't my impression from that LSF/MM session, but once the
videos become available, folks can decide for themselves.

       	      		       	   	  - Ted

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-17  8:10   ` Christoph Hellwig
@ 2022-05-17  9:18     ` Javier González
  2022-05-18  8:00       ` Christoph Hellwig
  2022-05-17 15:34       ` Theodore Ts'o
  1 sibling, 1 reply; 56+ messages in thread
From: Javier González @ 2022-05-17  9:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pankaj Raghav, axboe, pankydev8, gost.dev, damien.lemoal,
	jiangbo.365, linux-nvme, linux-kernel, linux-block,
	linux-fsdevel, dm-devel, dsterba, linux-btrfs

On 17.05.2022 10:10, Christoph Hellwig wrote:
>I'm a little surprised about all this activity.
>
>I though the conclusion at LSF/MM was that for Linux itself there
>is very little benefit in supporting this scheme.  It will massively
>fragment the supported based of devices and applications, while only
>having the benefit of supporting some Samsung legacy devices.

I believed we had agreed that non-power-of-2 zoned devices was something
to explore. Let me summarize the 3 main points we covered at different
times at LSF/MM:

   - This is not for legacy Samsung ZNS devices. At least 4 other
     vendors have reported building non-power-of-2 ZNS devices to meet
     customer demands on removing holes in the address space. It seems
     like there will be more ZNS devices with size=capacity out there
     than with PO2 sizes. Block device and FS support is very desirable
     for these.

   - We also talked about how the capacity not being a PO2 is the one
     introducing the fragmentation, as applications that already worked
     with SMR HDDs will have to change their data placement policy. The
     size is just a construction, but the real work is adopting the
     capacity.

   - Besides the previous poit, the fragmentation will happen from the
     moment we have available devices. This is not a kernel-only issue.
     We have SMR, ZNS, and soon another spec for zone devices. I
     understood that as long as we do not break any existing support, we
     would be able to expend the zoned ecosystem in Linux.

>So my impression was that this work, while technically feasible, is
>rather useless.  So unless I missed something important I have no
>interest in supporting this in NVMe.

Does the above help you reconsidering your interest in supporting this
in NVMe?

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
  2022-05-16 16:54 ` Pankaj Raghav
@ 2022-05-17  8:10   ` Christoph Hellwig
  2022-05-17  9:18     ` Javier González
  2022-05-17 15:34       ` Theodore Ts'o
  0 siblings, 2 replies; 56+ messages in thread
From: Christoph Hellwig @ 2022-05-17  8:10 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: axboe, pankydev8, gost.dev, damien.lemoal, jiangbo.365,
	linux-nvme, linux-kernel, linux-block, linux-fsdevel, dm-devel,
	dsterba, hch, linux-btrfs

I'm a little surprised about all this activity.

I though the conclusion at LSF/MM was that for Linux itself there
is very little benefit in supporting this scheme.  It will massively
fragment the supported based of devices and applications, while only
having the benefit of supporting some Samsung legacy devices.

So my impression was that this work, while technically feasible, is
rather useless.  So unless I missed something important I have no
interest in supporting this in NVMe.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices
       [not found] <CGME20220516165418eucas1p2be592d9cd4b35f6b71d39ccbe87f3fef@eucas1p2.samsung.com>
@ 2022-05-16 16:54 ` Pankaj Raghav
  2022-05-17  8:10   ` Christoph Hellwig
  0 siblings, 1 reply; 56+ messages in thread
From: Pankaj Raghav @ 2022-05-16 16:54 UTC (permalink / raw)
  To: axboe, damien.lemoal, pankydev8, dsterba, hch
  Cc: p.raghav, gost.dev, jiangbo.365, linux-nvme, linux-kernel,
	linux-block, dm-devel, linux-fsdevel, linux-btrfs

- Background and Motivation:

The zone storage implementation in Linux, introduced since v4.10, first
targetted SMR drives which have a power of 2 (po2) zone size alignment
requirement. The po2 zone size was further imposed implicitly by the
block layer's blk_queue_chunk_sectors(), used to prevent IO merging
across chunks beyond the specified size, since v3.16 through commit
762380ad9322 ("block: add notion of a chunk size for request merging").
But this same general block layer po2 requirement for blk_queue_chunk_sectors()
was removed on v5.10 through commit 07d098e6bbad ("block: allow 'chunk_sectors'
to be non-power-of-2").

NAND, which is the media used in newer zoned storage devices, does not
naturally align to po2. In these devices, zone cap is not the same as the
po2 zone size. When the zone cap != zone size, then unmapped LBAs are
introduced to cover the space between the zone cap and zone size. po2
requirement does not make sense for these type of zone storage devices.
This patch series aims to remove these unmapped LBAs for zoned devices when
zone cap is npo2. This is done by relaxing the po2 zone size constraint
in the kernel and allowing zoned device with npo2 zone sizes if zone cap
== zone size.

Removing the po2 requirement from zone storage should be possible
now provided that no userspace regression and no performance regressions are
introduced. Stop-gap patches have been already merged into f2fs-tools to
proactively not allow npo2 zone sizes until proper support is added [0].
Additional kernel stop-gap patches are provided in this series for dm-zoned.
Support for npo2 zonefs and btrfs support is addressed in this series.

There was an effort previously [1] to add support to non po2 devices via
device level emulation but that was rejected with a final conclusion
to add support for non po2 zoned device in the complete stack[2].

- Patchset description:
This patchset aims at adding support to non power of 2 zoned devices in
the block layer, nvme layer, null blk and adds support to btrfs and
zonefs.

This round of patches **will not** support DM layer for non
power of 2 zoned devices. More about this in the future work section.

Patches 1-2 deals with removing the po2 constraint from the
block layer.

Patches 3-4 deals with removing the constraint from nvme zns.

Patches 5-9 adds support to btrfs for non po2 zoned devices.

Patch 10 removes the po2 constraint in ZoneFS

Patch 11-12 removes the po2 contraint in null blk

Patches 13 adds conditions to not allow non power of 2 devices in
DM.

The patch series is based on linux-next tag: next-20220502

- Performance:
PO2 zone sizes utilizes log and shifts instead of division when
determing alignment, zone number, etc. The same math cannot be used when
using a zoned device with non po2 zone size. Hence, to avoid any performance
regression on zoned devices with po2 zone sizes, the optimized math in the
hot paths has been retained with branching.

The performance was measured using null blk for regression
and the results have been posted in the appropriate commit log. No
performance regression was noticed.

- Testing
With respect to testing we need to tackle two things: one for regression
on po2 zoned device and progression on non po2 zoned devices.

kdevops (https://github.com/mcgrof/kdevops) was extensively used to
automate the testing for blktests and (x)fstests for btrfs changes. The
known failures were excluded during the test based on the baseline
v5.17.0-rc7

-- regression
Emulated zoned device with zone size =128M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Changes were tested with the following profile in QEMU:
[btrfs_simple_zns]
TEST_DIR=<dir>
SCRATCH_MNT=<mnt>
FSTYP=btrfs
MKFS_OPTIONS="-f -d single -m single"
TEST_DEV=<dev>
SCRATCH_DEV_POOL=<dev-pool>

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests-nullblk.sh and zonefs-tests.sh from zonefs-tools were run
with no failures.

nullblk:
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified if dm-zoned successfully mounts without any
error.

-- progression
Emulated zoned device with zone size = 96M , nr_zones = 10000

Block and nvme zns:
blktests were run with no new failures

Btrfs:
Same profile as po2 zone size was used.

Many tests in xfstests for btrfs included dm-flakey and some tests
required dm-linear. As they are not supported at the moment for non
po2 devices, those **tests were excluded for non po2 devices**.

No new failures were observed in btrfs, generic and shared test suite

ZoneFS:
zonefs-tests.sh from zonefs-tools were run with no failures.

nullblk:
A new section was added to cover non po2 devices:

section14()
{
       conv_pcnt=10
       zone_size=3
       zone_capacity=3
       max_open=${set_max_open}
       zbd_test_opts+=("-o ${max_open}")
}
t/zbd/run-tests-against-nullb from fio was run with no failures.

DM:
It was verified that dm-zoned does not mount.

- Tools:
Some tools had to be updated to support non po2 devices. Once these
patches are accepted in the kernel, these tool updates will also be
upstreamed.
* btrfs-prog: https://github.com/Panky-codes/btrfs-progs/tree/remove-po2-btrfs
* blkzone: https://github.com/Panky-codes/util-linux/tree/remove-po2
* zonefs-tools: https://github.com/Panky-codes/zonefs-tools/tree/remove-po2

- Future work
To reduce the amount of changes and testing, support for DM was
excluded in this round of patches. The plan is to add support to F2FS
and DM in the forthcoming future.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test&id=6afcf6493578e77528abe65ab8b12f3e1c16749f
[1] https://lore.kernel.org/all/20220310094725.GA28499@lst.de/T/
[2] https://lore.kernel.org/all/20220315135245.eqf4tqngxxb7ymqa@unifi/

Changes since v1:
- Put the function declaration and its usage in the same commit (Bart)
- Remove bdev_zone_aligned function (Bart)
- Change the name from blk_queue_zone_aligned to blk_queue_is_zone_start
  (Damien)
- q is never null in from bdev_get_queue (Damien)
- Add condition during bringup and check for zsze == zcap for npo2
  drives (Damien)
- Rounddown operation should be made generic to work in 32 bits arch
  (bart)
- Add comments where generic calculation is directly used instead having
  special handling for po2 zone sizes (Hannes)
- Make the minimum zone size alignment requirement for btrfs to be 1M
  instead of BTRFS_STRIPE_LEN(David)

Changes since v2:
- Minor formatting changes

Changes since v3:
- Make superblock mirror align with the existing superblock log offsets
  (David)
- DM change return value and remove extra newline
- Optimize null blk zone index lookup with shift for po2 zone size

Luis Chamberlain (1):
  dm-zoned: ensure only power of 2 zone sizes are allowed

Pankaj Raghav (12):
  block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2
    zsze
  block: allow blk-zoned devices to have non-power-of-2 zone size
  nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
  nvmet: Allow ZNS target to support non-power_of_2 zone sizes
  btrfs: zoned: Cache superblock location in btrfs_zoned_device_info
  btrfs: zoned: Make sb_zone_number function non power of 2 compatible
  btrfs: zoned: use generic btrfs zone helpers to support npo2 zoned
    devices
  btrfs:zoned: make sb for npo2 zone devices align with sb log offsets
  btrfs: zoned: relax the alignment constraint for zoned devices
  zonefs: allow non power of 2 zoned devices
  null_blk: allow non power of 2 zoned devices
  null_blk: use zone_size_sects_shift for power of 2 zoned devices

 block/blk-core.c                  |   3 +-
 block/blk-zoned.c                 |  40 +++++--
 drivers/block/null_blk/main.c     |   5 +-
 drivers/block/null_blk/null_blk.h |   6 +
 drivers/block/null_blk/zoned.c    |  20 ++--
 drivers/md/dm-zone.c              |  12 ++
 drivers/nvme/host/zns.c           |  24 ++--
 drivers/nvme/target/zns.c         |   2 +-
 fs/btrfs/volumes.c                |  24 ++--
 fs/btrfs/zoned.c                  | 191 +++++++++++++++++++++---------
 fs/btrfs/zoned.h                  |  44 ++++++-
 fs/zonefs/super.c                 |   6 +-
 fs/zonefs/zonefs.h                |   1 -
 include/linux/blkdev.h            |  37 +++++-
 14 files changed, 303 insertions(+), 112 deletions(-)

-- 
2.25.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

end of thread, other threads:[~2022-05-25  7:23 UTC | newest]

Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20220516133922eucas1p1c891cd1d82539b4e792acb5d1aa74444@eucas1p1.samsung.com>
2022-05-16 13:39 ` [PATCH v4 00/13] support non power of 2 zoned devices Pankaj Raghav
2022-05-16 13:39   ` [dm-devel] " Pankaj Raghav
     [not found]   ` <CGME20220516133924eucas1p1817f306e3f2442088bf49ab513657cbe@eucas1p1.samsung.com>
2022-05-16 13:39     ` [PATCH v4 01/13] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze Pankaj Raghav
2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
2022-05-16 13:54       ` Damien Le Moal
2022-05-16 13:54         ` [dm-devel] " Damien Le Moal
     [not found]   ` <CGME20220516133925eucas1p1414fab2cfa7da1d6258315cbd33e1685@eucas1p1.samsung.com>
2022-05-16 13:39     ` [PATCH v4 02/13] block: allow blk-zoned devices to have non-power-of-2 zone size Pankaj Raghav
2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
2022-05-16 14:00       ` Damien Le Moal
2022-05-16 14:00         ` [dm-devel] " Damien Le Moal
     [not found]   ` <CGME20220516133926eucas1p15c7ba425b67ce4ac824c6bd3263e2dd4@eucas1p1.samsung.com>
2022-05-16 13:39     ` [PATCH v4 03/13] nvme: zns: Allow ZNS drives that have non-power_of_2 " Pankaj Raghav
2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
2022-05-16 14:02       ` Damien Le Moal
2022-05-16 14:02         ` Damien Le Moal
     [not found]   ` <CGME20220516133927eucas1p1bab57e07c14c1194705e254afdd5d346@eucas1p1.samsung.com>
2022-05-16 13:39     ` [PATCH v4 04/13] nvmet: Allow ZNS target to support non-power_of_2 zone sizes Pankaj Raghav
2022-05-16 13:39       ` [dm-devel] " Pankaj Raghav
2022-05-16 14:15   ` [dm-devel] [PATCH v4 00/13] support non power of 2 zoned devices Damien Le Moal
2022-05-16 14:15     ` Damien Le Moal
2022-05-16 14:33     ` notnand
     [not found] <CGME20220516165418eucas1p2be592d9cd4b35f6b71d39ccbe87f3fef@eucas1p2.samsung.com>
2022-05-16 16:54 ` Pankaj Raghav
2022-05-17  8:10   ` Christoph Hellwig
2022-05-17  9:18     ` Javier González
2022-05-18  8:00       ` Christoph Hellwig
2022-05-19 15:25         ` Javier González
2022-05-17 15:34     ` Theodore Ts'o
2022-05-17 15:34       ` Theodore Ts'o
2022-05-18 23:06       ` Luis Chamberlain
2022-05-18 23:06         ` Luis Chamberlain
2022-05-19  3:08       ` Damien Le Moal
2022-05-19  3:08         ` Damien Le Moal
2022-05-19  3:12         ` Luis Chamberlain
2022-05-19  3:12           ` Luis Chamberlain
2022-05-19  3:19           ` Damien Le Moal
2022-05-19  3:19             ` Damien Le Moal
2022-05-19  7:34             ` Johannes Thumshirn
2022-05-19  7:34               ` Johannes Thumshirn
2022-05-20  3:47               ` Damien Le Moal
2022-05-20  3:47                 ` Damien Le Moal
2022-05-20  6:07                 ` Hannes Reinecke
2022-05-20  6:07                   ` Hannes Reinecke
2022-05-20  6:27                   ` Javier González
2022-05-20  6:27                     ` Javier González
2022-05-20  6:41                     ` Damien Le Moal
2022-05-20  6:41                       ` Damien Le Moal
     [not found]                       ` <CGME20220520065941eucas1p105cf273ede995dc4bf92f3245fad09b1@eucas1p1.samsung.com>
2022-05-20  6:59                         ` Javier González
2022-05-20  6:59                           ` Javier González
2022-05-20  9:30                       ` Pankaj Raghav
2022-05-20  9:30                         ` Pankaj Raghav
2022-05-20 17:18                         ` David Sterba
2022-05-20 17:18                           ` David Sterba
2022-05-23  8:25                           ` Pankaj Raghav
2022-05-23  8:25                             ` Pankaj Raghav
2022-05-20  9:30                     ` Johannes Thumshirn
2022-05-20  9:30                       ` Johannes Thumshirn
     [not found]                       ` <CGME20220520101610eucas1p1822ca6014e2a1d55ae74476f83c4de1d@eucas1p1.samsung.com>
2022-05-20 10:16                         ` Javier González
2022-05-20 10:16                           ` Javier González

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.