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

From: Hannes Reinecke <hare@suse.de>
To: Damien Le Moal <Damien.LeMoal@hgst.com>, 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 07:57:36 +0200	[thread overview]
Message-ID: <5705F6D0.70604@suse.de> (raw)
In-Reply-To: <CC35A7E7-DF48-49F9-ABCF-B93A5B95DC99@hgst.com>

On 04/07/2016 03:56 AM, Damien Le Moal wrote:
> 
> 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.
> 
Sure.

Actually I was looking in blanking out this attribute entirely like
I did with the SCSI sysfs attributes, but as the 'queue' sysfs
directory is a blank kobj it's not easily done.
But yes, '0' seems like a reasonable value here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html