Re: [PATCH 6/9] block: Add 'zoned' sysfs queue attribute

From: Damien Le Moal <Damien.LeMoal@hgst.com>
To: Hannes Reinecke <hare@suse.de>, Jens Axboe <axboe@fb.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	Shaun Tancheff <shaun.tancheff@seagate.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Sathya Prakash <sathya.prakash@broadcom.com>
Subject: Re: [PATCH 6/9] block: Add 'zoned' sysfs queue attribute
Date: Thu, 7 Apr 2016 01:56:11 +0000	[thread overview]
Message-ID: <CC35A7E7-DF48-49F9-ABCF-B93A5B95DC99@hgst.com> (raw)
In-Reply-To: <1459764020-126038-7-git-send-email-hare@suse.de>

Hannes,

>Add a sysfs queue attribute 'zoned' to display the zone layout
>for zoned devices.
>
>Signed-off-by: Hannes Reinecke <hare@suse.de>
>---
> block/blk-sysfs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
>diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>index ff97091..748bb27 100644
>--- a/block/blk-sysfs.c
>+++ b/block/blk-sysfs.c
>@@ -244,6 +244,43 @@ static ssize_t queue_max_hw_sectors_show(struct request_queue *q, char *page)
> 	return queue_var_show(max_hw_sectors_kb, (page));
> }
> 
>+#ifdef CONFIG_BLK_DEV_ZONED
>+static ssize_t queue_zoned_show(struct request_queue *q, char *page)
>+{
>+	struct rb_node *node;
>+	struct blk_zone *zone;
>+	ssize_t offset = 0, end = 0;
>+	size_t size = 0, num = 0;
>+	enum blk_zone_type type = BLK_ZONE_TYPE_UNKNOWN;
>+
>+	for (node = rb_first(&q->zones); node; node = rb_next(node)) {
>+		zone = rb_entry(node, struct blk_zone, node);
>+		if (zone->type != type ||
>+		    zone->len != size ||
>+		    end != zone->start) {
>+			if (size != 0)
>+				offset += sprintf(page + offset, "%zu\n", num);
>+			/* We can only store one page ... */
>+			if (offset + 42 > PAGE_SIZE) {
>+				offset += sprintf(page + offset, "...\n");
>+				return offset;
>+			}
>+			size = zone->len;
>+			type = zone->type;
>+			offset += sprintf(page + offset, "%zu %zu %d ",
>+					  zone->start, size, type);
>+			num = 0;
>+			end = zone->start + size;
>+		} else
>+			end += zone->len;
>+		num++;
>+	}
>+	if (num > 0)
>+		offset += sprintf(page + offset, "%zu\n", num);
>+	return offset > 0 ? offset : -EINVAL;
>+}
>+#endif

With this, an application reading the “zoned” file for a non-SMR disk
will get a -EINVAL error. Not really super nice. Could we just have the
zoned files contain a single “0” for non-SMR disks ? Or at least have the
file empty and read return 0 Bytes. That would allow applications to easily
and cleanly detect if they are dealing with a SMR disk (or not) instead of
assuming that “-EINVAL” means “not SMR”, which seems very ugly to me.

Best.

--
Damien Le Moal, Ph.D.
Sr Manager, System Software Group, WW Research,
HGST, a Western Digital company
Damien.LeMoal@hgst.com
Tel: (+81) 0466-98-3593 (Ext. 51-3593)
1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan
www.hgst.com <http://www.hgst.com>
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.