All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] block: Check partition alignment on zoned block devices
@ 2016-12-01  0:22 Damien Le Moal
  2016-12-01  1:40 ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2016-12-01  0:22 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Shaun Tancheff, Damien Le Moal

Both blkdev_report_zones and blkdev_reset_zones can operate on a partition of
a zoned block device. However, the first and last zones reported for a
partition make sense only if the partition start sector and size are aligned
on the device zone size. The same applies for zone reset. Resetting the first
or the last zone of a partition straddling zones may impact neighboring
partitions. Finally, if a partition start sector is not at the beginning of a
sequential zone, it will be impossible to write to the first sectors of the
partition on a host-managed device.
Avoid all these problems and incoherencies by ignoring partitions that are not
zone aligned.

Note: Even with CONFIG_BLK_DEV_ZONED disabled, bdev_is_zoned() will report the
correct disk zoning type (host-aware, host-managed or none) but
bdev_zone_size() will always return 0 for zoned block devices (i.e. the zone
size is unknown). So test this as a way to ensure that a zoned block device is
being handled as such. As a result, for a host-aware devices, unaligned zone
partitions will be accepted with CONFIG_BLK_DEV_ZONED disabled. That is, the
disk will be treated as a regular block device (as it should). If zoned block
device support is enabled, only aligned partitions will be accepted.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/partition-generic.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/block/partition-generic.c b/block/partition-generic.c
index 71d9ed9..3bb9f13 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -430,6 +430,27 @@ static int drop_partitions(struct gendisk *disk, struct block_device *bdev)
 	return 0;
 }
 
+static bool part_zone_aligned(struct gendisk *disk,
+			      struct block_device *bdev,
+			      sector_t from, sector_t size)
+{
+	sector_t zone_size = bdev_zone_size(bdev);
+
+	/* Check partition start alignement */
+	if (from & (zone_size - 1))
+		return false;
+
+	/*
+	 * Check partition end alignement.
+	 * Ignore eventual last smaller zone.
+	 */
+	if ((from + size) < get_capacity(disk) &&
+	    (size & (zone_size - 1)))
+		return false;
+
+	return true;
+}
+
 int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
 {
 	struct parsed_partitions *state = NULL;
@@ -529,6 +550,25 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
 			}
 		}
 
+		/*
+		 * On a zoned block device, partitions should be aligned on the
+		 * device zone size (i.e. zone boundary crossing not allowed).
+		 * Otherwise, resetting the write pointer of the last zone of
+		 * one partition may impact the following partition. Note that
+		 * bdev_is_zoned() always returns the correct device zoning type
+		 * (host-aware, host-managed or none), even when support for
+		 * zoned block devices is disabled. However, in this last case,
+		 * the zone size will be reported as 0.
+		 */
+		if (bdev_is_zoned(bdev) &&
+		    bdev_zone_size(bdev) &&
+		    !part_zone_aligned(disk, bdev, from, size)) {
+			printk(KERN_WARNING
+			       "%s: p%d start %llu is not zone unaligned\n",
+			       disk->disk_name, p, (unsigned long long) from);
+			continue;
+		}
+
 		part = add_partition(disk, p, from, size,
 				     state->parts[p].flags,
 				     &state->parts[p].info);
-- 
2.7.4

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

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

* Re: [PATCH v2] block: Check partition alignment on zoned block devices
  2016-12-01  0:22 [PATCH v2] block: Check partition alignment on zoned block devices Damien Le Moal
@ 2016-12-01  1:40 ` Jens Axboe
  2016-12-01  2:38   ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2016-12-01  1:40 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-block, Shaun Tancheff

On 11/30/2016 05:22 PM, Damien Le Moal wrote:
> Both blkdev_report_zones and blkdev_reset_zones can operate on a partition of
> a zoned block device. However, the first and last zones reported for a
> partition make sense only if the partition start sector and size are aligned
> on the device zone size. The same applies for zone reset. Resetting the first
> or the last zone of a partition straddling zones may impact neighboring
> partitions. Finally, if a partition start sector is not at the beginning of a
> sequential zone, it will be impossible to write to the first sectors of the
> partition on a host-managed device.
> Avoid all these problems and incoherencies by ignoring partitions that are not
> zone aligned.
> 
> Note: Even with CONFIG_BLK_DEV_ZONED disabled, bdev_is_zoned() will report the
> correct disk zoning type (host-aware, host-managed or none) but
> bdev_zone_size() will always return 0 for zoned block devices (i.e. the zone
> size is unknown). So test this as a way to ensure that a zoned block device is
> being handled as such. As a result, for a host-aware devices, unaligned zone
> partitions will be accepted with CONFIG_BLK_DEV_ZONED disabled. That is, the
> disk will be treated as a regular block device (as it should). If zoned block
> device support is enabled, only aligned partitions will be accepted.

This looks better, thanks. Are the zone sizes mandated by spec to be a
power-of-2?

-- 
Jens Axboe

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

* Re: [PATCH v2] block: Check partition alignment on zoned block devices
  2016-12-01  1:40 ` Jens Axboe
@ 2016-12-01  2:38   ` Damien Le Moal
  2016-12-01  3:03     ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2016-12-01  2:38 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Shaun Tancheff


Jens,

On 12/1/16 10:40, Jens Axboe wrote:
> This looks better, thanks. Are the zone sizes mandated by spec to be a
> power-of-2?

No, the standards allow any zone size, and different sizes for the zones
too. However, the sd_zbc code down in the SCSI stack limits support to
HM & HA drives that have a power of 2 zone size, with all zones of the
same size, except for an eventual smaller last zone (Seagate drives have
that). This restriction was necessary so that limits.chunk_sectors can
be used to avoid BIO spawning zones.

See 89d9475610771b5e5fe1879075f0fc9ba6e3755f:

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
new file mode 100644
index 0000000..16d3fa6
--- /dev/null
+++ b/drivers/scsi/sd_zbc.c
@@ -0,0 +1,642 @@
+/*
+ * SCSI Zoned Block commands
...
+       if (!is_power_of_2(zone_blocks)) {
+               if (sdkp->first_scan)
+                       sd_printk(KERN_NOTICE, sdkp,
+                                 "Devices with non power of 2 zone "
+                                 "size are not supported\n");
+               return -ENODEV;
+       }
+

Martin, Shaun and myself agreed on the restriction. All ZBC/ZAC disks on
the market today match these so there are no problems. And it is
unlikely that we will ever see weirdly sized SMR drives (customers
generally do not want that).

Best regards.

-- =

Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality N=
otice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or l=
egally privileged information of WDC and/or its affiliates, and are intende=
d solely for the use of the individual or entity to which they are addresse=
d. If you are not the intended recipient, any disclosure, copying, distribu=
tion or any action taken or omitted to be taken in reliance on it, is prohi=
bited. If you have received this e-mail in error, please notify the sender =
immediately and delete the e-mail in its entirety from your system.

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

* Re: [PATCH v2] block: Check partition alignment on zoned block devices
  2016-12-01  2:38   ` Damien Le Moal
@ 2016-12-01  3:03     ` Jens Axboe
  2016-12-01  3:42       ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2016-12-01  3:03 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-block, Shaun Tancheff

On 11/30/2016 07:38 PM, Damien Le Moal wrote:
> 
> Jens,
> 
> On 12/1/16 10:40, Jens Axboe wrote:
>> This looks better, thanks. Are the zone sizes mandated by spec to be a
>> power-of-2?
> 
> No, the standards allow any zone size, and different sizes for the zones
> too. However, the sd_zbc code down in the SCSI stack limits support to
> HM & HA drives that have a power of 2 zone size, with all zones of the
> same size, except for an eventual smaller last zone (Seagate drives have
> that). This restriction was necessary so that limits.chunk_sectors can
> be used to avoid BIO spawning zones.
> 
> See 89d9475610771b5e5fe1879075f0fc9ba6e3755f:
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> new file mode 100644
> index 0000000..16d3fa6
> --- /dev/null
> +++ b/drivers/scsi/sd_zbc.c
> @@ -0,0 +1,642 @@
> +/*
> + * SCSI Zoned Block commands
> ...
> +       if (!is_power_of_2(zone_blocks)) {
> +               if (sdkp->first_scan)
> +                       sd_printk(KERN_NOTICE, sdkp,
> +                                 "Devices with non power of 2 zone "
> +                                 "size are not supported\n");
> +               return -ENODEV;
> +       }
> +
> 
> Martin, Shaun and myself agreed on the restriction. All ZBC/ZAC disks on
> the market today match these so there are no problems. And it is
> unlikely that we will ever see weirdly sized SMR drives (customers
> generally do not want that).

I'm fine with that, my only concern is that part_zone_aligned() assumes
this, and it's a bit fragile. If we remove the power-of-2 restriction.
Not sure what the best way to fix it is. Ideally it'd have a
WARN_ON_ONCE() and a fallback to a modulo calculation instead of the
AND.

-- 
Jens Axboe

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

* Re: [PATCH v2] block: Check partition alignment on zoned block devices
  2016-12-01  3:03     ` Jens Axboe
@ 2016-12-01  3:42       ` Damien Le Moal
  2016-12-01  3:59         ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2016-12-01  3:42 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Shaun Tancheff


Jens,

On 12/1/16 12:03, Jens Axboe wrote:
>> Martin, Shaun and myself agreed on the restriction. All ZBC/ZAC disks on
>> the market today match these so there are no problems. And it is
>> unlikely that we will ever see weirdly sized SMR drives (customers
>> generally do not want that).
> 
> I'm fine with that, my only concern is that part_zone_aligned() assumes
> this, and it's a bit fragile. If we remove the power-of-2 restriction.
> Not sure what the best way to fix it is. Ideally it'd have a
> WARN_ON_ONCE() and a fallback to a modulo calculation instead of the
> AND.

I can have a crack at it. Should I resubmit a new version ?

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

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

* Re: [PATCH v2] block: Check partition alignment on zoned block devices
  2016-12-01  3:42       ` Damien Le Moal
@ 2016-12-01  3:59         ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2016-12-01  3:59 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-block, Shaun Tancheff


> On Nov 30, 2016, at 8:42 PM, Damien Le Moal <damien.lemoal@wdc.com> wrote:
> 
> 
> Jens,
> 
> On 12/1/16 12:03, Jens Axboe wrote:
>>> Martin, Shaun and myself agreed on the restriction. All ZBC/ZAC disks on
>>> the market today match these so there are no problems. And it is
>>> unlikely that we will ever see weirdly sized SMR drives (customers
>>> generally do not want that).
>> 
>> I'm fine with that, my only concern is that part_zone_aligned() assumes
>> this, and it's a bit fragile. If we remove the power-of-2 restriction.
>> Not sure what the best way to fix it is. Ideally it'd have a
>> WARN_ON_ONCE() and a fallback to a modulo calculation instead of the
>> AND.
> 
> I can have a crack at it. Should I resubmit a new version ?

That'd be great, thanks. 

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

end of thread, other threads:[~2016-12-01  3:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01  0:22 [PATCH v2] block: Check partition alignment on zoned block devices Damien Le Moal
2016-12-01  1:40 ` Jens Axboe
2016-12-01  2:38   ` Damien Le Moal
2016-12-01  3:03     ` Jens Axboe
2016-12-01  3:42       ` Damien Le Moal
2016-12-01  3:59         ` Jens Axboe

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.