Skip to contentSkip to conversation

block: support arbitrary zone size

PATCH Superseded Alexey Dobriyan 8 participants 10 messages Feb 10, 2020 – Feb 12, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 block: support arbitrary zone size

SK hynix is going to ship ZNS device with zone size not being power of 2.

---

 block/blk-settings.c           |    4 +---
 block/blk-zoned.c              |   10 +++++-----
 drivers/block/null_blk_main.c  |   11 ++++++-----
 drivers/block/null_blk_zoned.c |   10 ++--------
 4 files changed, 14 insertions(+), 21 deletions(-)
block/blk-settings.c +1 -3
@@ -206,15 +206,13 @@ EXPORT_SYMBOL(blk_queue_max_hw_sectors);
  *
  * Description:
  *    If a driver doesn't want IOs to cross a given chunk size, it can set
- *    this limit and prevent merging across chunks. Note that the chunk size
- *    must currently be a power-of-2 in sectors. Also note that the block
+ *    this limit and prevent merging across chunks. Note that the block
  *    layer must accept a page worth of data at any offset. So if the
  *    crossing of chunks is a hard limitation in the driver, it must still be
  *    prepared to split single page bios.
  **/
 void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors)
 {
-	BUG_ON(!is_power_of_2(chunk_sectors));
 	q->limits.chunk_sectors = chunk_sectors;
 }
 EXPORT_SYMBOL(blk_queue_chunk_sectors);
block/blk-zoned.c +5 -5
@@ -83,7 +83,7 @@ unsigned int blkdev_nr_zones(struct gendisk *disk)
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+	return div64_u64(get_capacity(disk) + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);
 
@@ -363,14 +363,14 @@ 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 zoned device with length 0\n",
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		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",
drivers/block/null_blk_main.c +6 -5
@@ -187,7 +187,7 @@ MODULE_PARM_DESC(zoned, "Make device as a host-managed zoned block device. Defau
 
 static unsigned long g_zone_size = 256;
 module_param_named(zone_size, g_zone_size, ulong, S_IRUGO);
-MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Must be power-of-two: Default: 256");
+MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Default: 256");
 
 static unsigned int g_zone_nr_conv;
 module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
@@ -1641,10 +1641,11 @@ static int null_validate_conf(struct nullb_device *dev)
 	if (dev->queue_mode == NULL_Q_BIO)
 		dev->mbps = 0;
 
-	if (dev->zoned &&
-	    (!dev->zone_size || !is_power_of_2(dev->zone_size))) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
+	if (dev->zoned) {
+		if (dev->zone_size == 0) {
+			pr_err("zone_size must be positive\n");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
drivers/block/null_blk_zoned.c +2 -8
@@ -7,7 +7,7 @@
 
 static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
 {
-	return sect >> ilog2(dev->zone_size_sects);
+	return div64_u64(sect, dev->zone_size_sects * SECTOR_SIZE);
 }
 
 int null_zone_init(struct nullb_device *dev)
@@ -16,14 +16,8 @@ int null_zone_init(struct nullb_device *dev)
 	sector_t sector = 0;
 	unsigned int i;
 
-	if (!is_power_of_2(dev->zone_size)) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
-	}
-
 	dev->zone_size_sects = dev->zone_size << ZONE_SIZE_SHIFT;
-	dev->nr_zones = dev_size >>
-				(SECTOR_SHIFT + ilog2(dev->zone_size_sects));
+	dev->nr_zones = div64_u64(dev_size, dev->zone_size_sects * SECTOR_SIZE);
 	dev->zones = kvmalloc_array(dev->nr_zones, sizeof(struct blk_zone),
 			GFP_KERNEL | __GFP_ZERO);
 	if (!dev->zones)
#1 lore raw

PATCH v2 1/1 block: support arbitrary zone size

SK hynix is going to ship ZNS device with zone size not being power of 2.

---

	v2: fixup one ">>ilog2"/div conversion :-/
 block/blk-settings.c           |    4 +---
 block/blk-zoned.c              |   10 +++++-----
 drivers/block/null_blk_main.c  |   11 ++++++-----
 drivers/block/null_blk_zoned.c |   10 ++--------
 4 files changed, 14 insertions(+), 21 deletions(-)
block/blk-settings.c +1 -3
@@ -206,15 +206,13 @@ EXPORT_SYMBOL(blk_queue_max_hw_sectors);
  *
  * Description:
  *    If a driver doesn't want IOs to cross a given chunk size, it can set
- *    this limit and prevent merging across chunks. Note that the chunk size
- *    must currently be a power-of-2 in sectors. Also note that the block
+ *    this limit and prevent merging across chunks. Note that the block
  *    layer must accept a page worth of data at any offset. So if the
  *    crossing of chunks is a hard limitation in the driver, it must still be
  *    prepared to split single page bios.
  **/
 void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors)
 {
-	BUG_ON(!is_power_of_2(chunk_sectors));
 	q->limits.chunk_sectors = chunk_sectors;
 }
 EXPORT_SYMBOL(blk_queue_chunk_sectors);
block/blk-zoned.c +5 -5
@@ -83,7 +83,7 @@ unsigned int blkdev_nr_zones(struct gendisk *disk)
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+	return div64_u64(get_capacity(disk) + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);
 
@@ -363,14 +363,14 @@ 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 zoned device with length 0\n",
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		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",
drivers/block/null_blk_main.c +6 -5
@@ -187,7 +187,7 @@ MODULE_PARM_DESC(zoned, "Make device as a host-managed zoned block device. Defau
 
 static unsigned long g_zone_size = 256;
 module_param_named(zone_size, g_zone_size, ulong, S_IRUGO);
-MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Must be power-of-two: Default: 256");
+MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Default: 256");
 
 static unsigned int g_zone_nr_conv;
 module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
@@ -1641,10 +1641,11 @@ static int null_validate_conf(struct nullb_device *dev)
 	if (dev->queue_mode == NULL_Q_BIO)
 		dev->mbps = 0;
 
-	if (dev->zoned &&
-	    (!dev->zone_size || !is_power_of_2(dev->zone_size))) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
+	if (dev->zoned) {
+		if (dev->zone_size == 0) {
+			pr_err("zone_size must be positive\n");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
drivers/block/null_blk_zoned.c +2 -8
@@ -7,7 +7,7 @@
 
 static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
 {
-	return sect >> ilog2(dev->zone_size_sects);
+	return div64_u64(sect, dev->zone_size_sects);
 }
 
 int null_zone_init(struct nullb_device *dev)
@@ -16,14 +16,8 @@ int null_zone_init(struct nullb_device *dev)
 	sector_t sector = 0;
 	unsigned int i;
 
-	if (!is_power_of_2(dev->zone_size)) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
-	}
-
 	dev->zone_size_sects = dev->zone_size << ZONE_SIZE_SHIFT;
-	dev->nr_zones = dev_size >>
-				(SECTOR_SHIFT + ilog2(dev->zone_size_sects));
+	dev->nr_zones = div64_u64(dev_size, dev->zone_size_sects * SECTOR_SIZE);
 	dev->zones = kvmalloc_array(dev->nr_zones, sizeof(struct blk_zone),
 			GFP_KERNEL | __GFP_ZERO);
 	if (!dev->zones)
#2 lore raw
Quoted 3 lines · Alexey Dobriyan

SK hynix is going to ship ZNS device with zone size not being power of 2.

In addition to Bart and Keith comments, I will add a clear NACK on this, at least for this patch as is. Details below inline.

Quoted 19 lines

---

	v2: fixup one ">>ilog2"/div conversion :-/
 block/blk-settings.c           |    4 +---
 block/blk-zoned.c              |   10 +++++-----
 drivers/block/null_blk_main.c  |   11 ++++++-----
 drivers/block/null_blk_zoned.c |   10 ++--------
 4 files changed, 14 insertions(+), 21 deletions(-)
block/blk-settings.c +1 -2
@@ -206,15 +206,13 @@ EXPORT_SYMBOL(blk_queue_max_hw_sectors);
  *
  * Description:
  *    If a driver doesn't want IOs to cross a given chunk size, it can set
- *    this limit and prevent merging across chunks. Note that the chunk size
- *    must currently be a power-of-2 in sectors. Also note that the block
+ *    this limit and prevent merging across chunks. Note that the block

block/blk-settings.c EXPORT_SYMBOL(blk_queue_max_hw_sectors); · line 218

Quoted 10 lines
block/blk-settings.c +0 -1
  *    layer must accept a page worth of data at any offset. So if the
  *    crossing of chunks is a hard limitation in the driver, it must still be
  *    prepared to split single page bios.
  **/
 void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors)
 {
-	BUG_ON(!is_power_of_2(chunk_sectors));
 	q->limits.chunk_sectors = chunk_sectors;
 }
 EXPORT_SYMBOL(blk_queue_chunk_sectors);

Chunk sectors is used by the entire bio/request merging and splitting code. For zoned devices, this ensures that requests do not cross over zones boundaries due to merges. All that code relies on power of 2 arithmetics (bit shifts), so allowing a non power of 2 value would mean changing all of that. That is the hot path ! adding multiplications and divisions will slow down all devices, including non zoned ones.

Furthermore, chunk sectors is also used to indicate the stripe size of software or hardware raid arrays for use by file systems and device mappers to do optimizations and also avoid merging of bios/request accross device stripe boundaries. Not enforcing a power of 2 here also potentially breaks all of that.

block/blk-zoned.c unsigned int blkdev_nr_zones(struct gendisk *disk) · line 88

Quoted 10 lines
block/blk-zoned.c +1 -1
@@ -83,7 +83,7 @@ unsigned int blkdev_nr_zones(struct gendisk *disk)
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+	return div64_u64(get_capacity(disk) + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);

Sure, and this is the kind of modification that will be needed all over the hot path for bio/request merging checks. Not nice at all. Slow down for everything.

Quoted 10 lines
block/blk-zoned.c +2 -3
@@ -363,14 +363,14 @@ 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 zoned device with length 0\n",

block/blk-zoned.c static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, · line 376

Quoted 10 lines
block/blk-zoned.c +2 -1
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		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",

You only have these 2 changes for the generic block layer part, but there are a lot more to add. Anything that has ilog2(chunk_sectors) (e.g. blk_queue_zone_no()) needs to be changed. mq-deadline and zone write locking handling, and all bio/request split & merge checks handling. And again, even with such changes, allowing chunk_sectors to not be a power of 2 will cause problems for non-zoned use cases such as RAID and in all file systems relying on that value to be a power of 2.

Quoted 45 lines
drivers/block/null_blk_main.c +6 -5
@@ -187,7 +187,7 @@ MODULE_PARM_DESC(zoned, "Make device as a host-managed zoned block device. Defau
 
 static unsigned long g_zone_size = 256;
 module_param_named(zone_size, g_zone_size, ulong, S_IRUGO);
-MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Must be power-of-two: Default: 256");
+MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Default: 256");
 
 static unsigned int g_zone_nr_conv;
 module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
@@ -1641,10 +1641,11 @@ static int null_validate_conf(struct nullb_device *dev)
 	if (dev->queue_mode == NULL_Q_BIO)
 		dev->mbps = 0;
 
-	if (dev->zoned &&
-	    (!dev->zone_size || !is_power_of_2(dev->zone_size))) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
+	if (dev->zoned) {
+		if (dev->zone_size == 0) {
+			pr_err("zone_size must be positive\n");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
drivers/block/null_blk_zoned.c +1 -3
@@ -7,7 +7,7 @@
 
 static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
 {
-	return sect >> ilog2(dev->zone_size_sects);
+	return div64_u64(sect, dev->zone_size_sects);
 }
 
 int null_zone_init(struct nullb_device *dev)
@@ -16,14 +16,8 @@ int null_zone_init(struct nullb_device *dev)
 	sector_t sector = 0;
 	unsigned int i;
 
-	if (!is_power_of_2(dev->zone_size)) {
-		pr_err("zone_size must be power-of-two\n");

drivers/block/null_blk_zoned.c int null_zone_init(struct nullb_device *dev) · line 23

Quoted 10 lines
drivers/block/null_blk_zoned.c +1 -5
-		return -EINVAL;
-	}
-
 	dev->zone_size_sects = dev->zone_size << ZONE_SIZE_SHIFT;
-	dev->nr_zones = dev_size >>
-				(SECTOR_SHIFT + ilog2(dev->zone_size_sects));
+	dev->nr_zones = div64_u64(dev_size, dev->zone_size_sects * SECTOR_SIZE);
 	dev->zones = kvmalloc_array(dev->nr_zones, sizeof(struct blk_zone),
 			GFP_KERNEL | __GFP_ZERO);
 	if (!dev->zones)

And beyond null_blk, other drivers are affected:
* DM core, dm-linear, dm-flakey and dm-zoned
* f2fs and zonefs file systems will not work as is on such drive. Ongoing brtfs work is affected too.
* probably some things get broken in the scsi stack too.
* And user space will suffer badly too, at the very least all user space tools associated with file systems and DM format/check and sys-utils.

Signature · 2 lines
Damien Le Moal
Western Digital Research
#3 lore raw
In reply to Alexey Dobriyan

Alexey,

Quoted 1 line

SK hynix is going to ship ZNS device with zone size not being power of 2.

That is going to wreak havoc all over the place. We have always stated to device vendors that Linux only supports block and zone sizes that are powers of two.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
#4 lore raw
In reply to Alexey Dobriyan
Quoted 4 lines · Alexey Dobriyan

SK hynix is going to ship ZNS device with zone size not being power of 2.

---

Nack from here as well. The power of 2 is a strict requirement. All SMR HDDs Zone Sizes are a power of two, and ZNS SSDs should be as well if they want Linux eco-system support.

The ZNS specification is specifically written to accommodate this. It has two type of zone sizes, the Zone Size (which is what should be a power of two) and the Zone Capacity (that is the writable capacity of the zone). For Linux support, your ZNS SSD implemention must have a zone size that is power of two, and instead use the Zone Capacity field to be the "Zone Size" that you have now. Then it works as expected, making both the host and device happy. The host gets power of two, and device can communicate the non-power of two writeable LBAs within the zone.

Best, Matias

#5 lore raw
Quoted 4 lines · Matias Bjørling

The ZNS specification is specifically written to accommodate this. It has two type of zone sizes, the Zone Size (which is what should be a power of two) and the Zone Capacity (that is the writable capacity of the zone).

Ouch, what an embarassing patch I've sent.
For some reason holes in LBA space completely escaped my mind.

Alexey

#6 lore raw
In reply to Alexey Dobriyan
Quoted 106 lines · Alexey Dobriyan

SK hynix is going to ship ZNS device with zone size not being power of 2.

---

	v2: fixup one ">>ilog2"/div conversion :-/
block/blk-settings.c           |    4 +---
block/blk-zoned.c              |   10 +++++-----
drivers/block/null_blk_main.c  |   11 ++++++-----
drivers/block/null_blk_zoned.c |   10 ++--------
4 files changed, 14 insertions(+), 21 deletions(-)
block/blk-settings.c +1 -3
@@ -206,15 +206,13 @@ EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 *
 * Description:
 *    If a driver doesn't want IOs to cross a given chunk size, it can set
- *    this limit and prevent merging across chunks. Note that the chunk size
- *    must currently be a power-of-2 in sectors. Also note that the block
+ *    this limit and prevent merging across chunks. Note that the block
 *    layer must accept a page worth of data at any offset. So if the
 *    crossing of chunks is a hard limitation in the driver, it must still be
 *    prepared to split single page bios.
 **/
 void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors)
 {
-	BUG_ON(!is_power_of_2(chunk_sectors));
 	q->limits.chunk_sectors = chunk_sectors;
 }
 EXPORT_SYMBOL(blk_queue_chunk_sectors);
block/blk-zoned.c +5 -5
@@ -83,7 +83,7 @@ unsigned int blkdev_nr_zones(struct gendisk *disk)
 
 	if (!blk_queue_is_zoned(disk->queue))
 		return 0;
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+	return div64_u64(get_capacity(disk) + zone_sectors - 1, zone_sectors);
 }
 EXPORT_SYMBOL_GPL(blkdev_nr_zones);
 
@@ -363,14 +363,14 @@ 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 zoned device with length 0\n",
+				disk->disk_name);
 			return -ENODEV;
 		}
 
 		args->zone_sectors = zone->len;
-		args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+		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",
drivers/block/null_blk_main.c +6 -5
@@ -187,7 +187,7 @@ MODULE_PARM_DESC(zoned, "Make device as a host-managed zoned block device. Defau
 
 static unsigned long g_zone_size = 256;
 module_param_named(zone_size, g_zone_size, ulong, S_IRUGO);
-MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Must be power-of-two: Default: 256");
+MODULE_PARM_DESC(zone_size, "Zone size in MB when block device is zoned. Default: 256");
 
 static unsigned int g_zone_nr_conv;
 module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
@@ -1641,10 +1641,11 @@ static int null_validate_conf(struct nullb_device *dev)
 	if (dev->queue_mode == NULL_Q_BIO)
 		dev->mbps = 0;
 
-	if (dev->zoned &&
-	    (!dev->zone_size || !is_power_of_2(dev->zone_size))) {
-		pr_err("zone_size must be power-of-two\n");
-		return -EINVAL;
+	if (dev->zoned) {
+		if (dev->zone_size == 0) {
+			pr_err("zone_size must be positive\n");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
drivers/block/null_blk_zoned.c +1 -3
@@ -7,7 +7,7 @@
 
 static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
 {
-	return sect >> ilog2(dev->zone_size_sects);
+	return div64_u64(sect, dev->zone_size_sects);
 }
 
 int null_zone_init(struct nullb_device *dev)
@@ -16,14 +16,8 @@ int null_zone_init(struct nullb_device *dev)
 	sector_t sector = 0;
 	unsigned int i;
 
-	if (!is_power_of_2(dev->zone_size)) {
-		pr_err("zone_size must be power-of-two\n");

drivers/block/null_blk_zoned.c int null_zone_init(struct nullb_device *dev) · line 23

Quoted 10 lines
drivers/block/null_blk_zoned.c +1 -5
-		return -EINVAL;
-	}
-
 	dev->zone_size_sects = dev->zone_size << ZONE_SIZE_SHIFT;
-	dev->nr_zones = dev_size >>
-				(SECTOR_SHIFT + ilog2(dev->zone_size_sects));
+	dev->nr_zones = div64_u64(dev_size, dev->zone_size_sects * SECTOR_SIZE);
 	dev->zones = kvmalloc_array(dev->nr_zones, sizeof(struct blk_zone),
 			GFP_KERNEL | __GFP_ZERO);
 	if (!dev->zones)

Same response from here. You will need to add some changes to support arbitrary zone capacities, but the zone size needs to be a power-of-2 to plug into the zoned framework.

Javier

#7 lore raw
In reply to Alexey Dobriyan
Quoted 1 line · Alexey Dobriyan

SK hynix is going to ship ZNS device with zone size not being power of 2.

NAK. We clearly stated we require power of two zone sizes in Linux. (Not Zone Capacities for ZNS, though - which is the whole point the zone capacity concept was added).

#8 lore raw
In reply to Alexey Dobriyan

block/blk-zoned.c unsigned int blkdev_nr_zones(struct gendisk *disk) · line 86

Quoted 2 lines · Alexey Dobriyan
block/blk-zoned.c +1 -1
-	return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+	return div64_u64(get_capacity(disk) + zone_sectors - 1, zone_sectors);

This kind of change impacts the performance negatively of ZNS devices for which the zone size is a power of two. I don't think it's fair to make others pay a price for a feature that is only needed for one single device.

Bart.

#9 lore raw
In reply to Alexey Dobriyan

block/blk-settings.c EXPORT_SYMBOL(blk_queue_max_hw_sectors); · line 217

Quoted 5 lines · Alexey Dobriyan

void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors) {

block/blk-settings.c +0 -1
-	BUG_ON(!is_power_of_2(chunk_sectors));
 	q->limits.chunk_sectors = chunk_sectors;
 }

This breaks blk_max_size_offset() if the value isn't a power of 2, but I wouldn't want to replace its cheap mask with an expensive divide operation anyway. Please keep the power-of-2 requirement.