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

From: Hannes Reinecke <hare@suse.de>
To: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Jens Axboe <axboe@fb.com>,
	linux-block@vger.kernel.org,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	Shaun Tancheff <shaun.tancheff@seagate.com>,
	Damien Le Moal <damien.lemoal@hgst.com>,
	linux-scsi@vger.kernel.org,
	Sathya Prakash <sathya.prakash@broadcom.com>
Subject: Re: [PATCH 6/9] block: Add 'zoned' sysfs queue attribute
Date: Fri, 15 Apr 2016 20:03:10 +0200	[thread overview]
Message-ID: <57112CDE.2080303@suse.de> (raw)
In-Reply-To: <571128C7.1060107@sandisk.com>

On 04/15/2016 07:45 PM, Bart Van Assche wrote:
> On 04/04/2016 03:00 AM, Hannes Reinecke wrote:
>> 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
>> +
>>   #define QUEUE_SYSFS_BIT_FNS(name, flag, neg)                \
>>   static ssize_t                                \
>>   queue_show_##name(struct request_queue *q, char *page)            \
>> @@ -468,6 +505,13 @@ static struct queue_sysfs_entry
>> queue_write_same_max_entry = {
>>       .show = queue_write_same_max_show,
>>   };
>>
>> +#ifdef CONFIG_BLK_DEV_ZONED
>> +static struct queue_sysfs_entry queue_zoned_entry = {
>> +    .attr = {.name = "zoned", .mode = S_IRUGO },
>> +    .show = queue_zoned_show,
>> +};
>> +#endif
>> +
>
> Hello Hannes,
>
> Have you considered to move the above definitions into a new file? That
> would allow to avoid two #ifdefs and to move the code that decides
> whether or not the above code gets built into block/Makefile.
>
Well, it's just these few lines, so I thought it easier to put it into 
the existing file.
But sure I can add a separate files for that.

> Additionally, have you considered to create one sysfs directory per zone
> instead of one sysfs attribute with all zone information? From
> Documentation/filesystems/sysfs.txt: "Attributes should be ASCII text
> files, preferably with only one value per file."
>
Yes, I have considered it.
But doing so would require me to add about 20k sysfs attributes.
For no apparent gain.
So I've settled on this condensed approach here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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