[PATCH 0/6] Support for zoned block devices

[PATCH 0/6] Support for zoned block devices

[Hannes Reinecke]

Hi Jens,

this patchset adds support for zoned block devices to the block layer.
Support for it is selected with the new 'BLK_DEV_ZONED' config option.

Patch has been made over Tejuns 'libata/for-4.8' repository.

As usual, comments and reviews are welcome.

Hannes Reinecke (6):
  blk-sysfs: Add 'chunk_sectors' to sysfs attributes
  block: update chunk_sectors in blk_stack_limits()
  block: Implement support for zoned block devices
  block: Add 'zoned' sysfs queue attribute
  block: Introduce BLKPREP_DONE
  block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

 block/Kconfig           |  9 +++++++
 block/Makefile          |  1 +
 block/blk-core.c        | 11 +++++++-
 block/blk-mq.c          |  1 +
 block/blk-settings.c    |  4 +++
 block/blk-sysfs.c       | 57 ++++++++++++++++++++++++++++++++++++++++
 block/blk-zoned.c       | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_lib.c |  4 +++
 include/linux/blk-mq.h  |  1 +
 include/linux/blkdev.h  | 48 +++++++++++++++++++++++++++++++++
 10 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 block/blk-zoned.c

-- 
1.8.5.6

[PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Hannes Reinecke]

The queue limits already have a 'chunk_sectors' setting, so
we should be presenting it via sysfs.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-sysfs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 9920596..73200b8 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -130,6 +130,11 @@ static ssize_t queue_physical_block_size_show(struct request_queue *q, char *pag
 	return queue_var_show(queue_physical_block_size(q), page);
 }
 
+static ssize_t queue_chunk_sectors_show(struct request_queue *q, char *page)
+{
+	return queue_var_show(q->limits.chunk_sectors, page);
+}
+
 static ssize_t queue_io_min_show(struct request_queue *q, char *page)
 {
 	return queue_var_show(queue_io_min(q), page);
@@ -438,6 +443,11 @@ static struct queue_sysfs_entry queue_physical_block_size_entry = {
 	.show = queue_physical_block_size_show,
 };
 
+static struct queue_sysfs_entry queue_chunk_sectors_entry = {
+	.attr = {.name = "chunk_sectors", .mode = S_IRUGO },
+	.show = queue_chunk_sectors_show,
+};
+
 static struct queue_sysfs_entry queue_io_min_entry = {
 	.attr = {.name = "minimum_io_size", .mode = S_IRUGO },
 	.show = queue_io_min_show,
@@ -528,6 +538,7 @@ static struct attribute *default_attrs[] = {
 	&queue_hw_sector_size_entry.attr,
 	&queue_logical_block_size_entry.attr,
 	&queue_physical_block_size_entry.attr,
+	&queue_chunk_sectors_entry.attr,
 	&queue_io_min_entry.attr,
 	&queue_io_opt_entry.attr,
 	&queue_discard_granularity_entry.attr,
-- 
1.8.5.6

[PATCH 2/6] block: update chunk_sectors in blk_stack_limits()

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 block/blk-settings.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index f679ae1..bfaf579 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -630,6 +630,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
 			t->discard_granularity;
 	}
 
+	if (b->chunk_sectors)
+		t->chunk_sectors = min_not_zero(t->chunk_sectors,
+						b->chunk_sectors);
+
 	return ret;
 }
 EXPORT_SYMBOL(blk_stack_limits);
-- 
1.8.5.6

[PATCH 3/6] block: Implement support for zoned block devices

[Hannes Reinecke]

Implement a RB-Tree holding the zone information and
add support functions for maintaining the RB-Tree.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/Kconfig          |  9 +++++++
 block/Makefile         |  1 +
 block/blk-core.c       |  5 ++++
 block/blk-zoned.c      | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h | 47 +++++++++++++++++++++++++++++++++
 5 files changed, 132 insertions(+)
 create mode 100644 block/blk-zoned.c

diff --git a/block/Kconfig b/block/Kconfig
index 0363cd7..8162312 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -113,6 +113,15 @@ config BLK_DEV_THROTTLING
 
 	See Documentation/cgroups/blkio-controller.txt for more information.
 
+config BLK_DEV_ZONED
+	bool "Zoned block device support"
+	default n
+	---help---
+	Block layer zoned block device support. This option enables
+	support for zoned block (ZAC/ZBC) devices.
+
+	Say yes here if you have a ZAC or ZBC storage device.
+
 config BLK_CMDLINE_PARSER
 	bool "Block device command line partition parser"
 	default n
diff --git a/block/Makefile b/block/Makefile
index 9eda232..bc9e62c 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_BLK_DEV_BSG)	+= bsg.o
 obj-$(CONFIG_BLK_DEV_BSGLIB)	+= bsg-lib.o
 obj-$(CONFIG_BLK_CGROUP)	+= blk-cgroup.o
 obj-$(CONFIG_BLK_DEV_THROTTLING)	+= blk-throttle.o
+obj-$(CONFIG_BLK_DEV_ZONED)	+= blk-zoned.o
 obj-$(CONFIG_IOSCHED_NOOP)	+= noop-iosched.o
 obj-$(CONFIG_IOSCHED_DEADLINE)	+= deadline-iosched.o
 obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched.o
diff --git a/block/blk-core.c b/block/blk-core.c
index 2475b1c7..e273194 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -588,6 +588,8 @@ void blk_cleanup_queue(struct request_queue *q)
 		blk_mq_free_queue(q);
 	percpu_ref_exit(&q->q_usage_counter);
 
+	blk_drop_zones(q);
+
 	spin_lock_irq(lock);
 	if (q->queue_lock != &q->__queue_lock)
 		q->queue_lock = &q->__queue_lock;
@@ -724,6 +726,9 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 #ifdef CONFIG_BLK_CGROUP
 	INIT_LIST_HEAD(&q->blkg_list);
 #endif
+#ifdef CONFIG_BLK_DEV_ZONED
+	q->zones = RB_ROOT;
+#endif
 	INIT_DELAYED_WORK(&q->delay_work, blk_delay_work);
 
 	kobject_init(&q->kobj, &blk_queue_ktype);
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
new file mode 100644
index 0000000..975e863
--- /dev/null
+++ b/block/blk-zoned.c
@@ -0,0 +1,70 @@
+/*
+ * Zoned block device handling
+ *
+ * Copyright (c) 2015, Hannes Reinecke
+ * Copyright (c) 2015, SUSE Linux GmbH
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/blkdev.h>
+#include <linux/rbtree.h>
+
+struct blk_zone *blk_lookup_zone(struct request_queue *q, sector_t lba)
+{
+	struct rb_root *root = &q->zones;
+	struct rb_node *node = root->rb_node;
+
+	while (node) {
+		struct blk_zone *zone = container_of(node, struct blk_zone,
+						     node);
+
+		if (lba < zone->start)
+			node = node->rb_left;
+		else if (lba >= zone->start + zone->len)
+			node = node->rb_right;
+		else
+			return zone;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(blk_lookup_zone);
+
+struct blk_zone *blk_insert_zone(struct request_queue *q, struct blk_zone *data)
+{
+	struct rb_root *root = &q->zones;
+	struct rb_node **new = &(root->rb_node), *parent = NULL;
+
+	/* Figure out where to put new node */
+	while (*new) {
+		struct blk_zone *this = container_of(*new, struct blk_zone,
+						     node);
+		parent = *new;
+		if (data->start + data->len <= this->start)
+			new = &((*new)->rb_left);
+		else if (data->start >= this->start + this->len)
+			new = &((*new)->rb_right);
+		else {
+			/* Return existing zone */
+			return this;
+		}
+	}
+	/* Add new node and rebalance tree. */
+	rb_link_node(&data->node, parent, new);
+	rb_insert_color(&data->node, root);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(blk_insert_zone);
+
+void blk_drop_zones(struct request_queue *q)
+{
+	struct rb_root *root = &q->zones;
+	struct blk_zone *zone, *next;
+
+	rbtree_postorder_for_each_entry_safe(zone, next, root, node) {
+		kfree(zone);
+	}
+	q->zones = RB_ROOT;
+}
+EXPORT_SYMBOL_GPL(blk_drop_zones);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3d9cf32..d5e3d8b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -249,6 +249,50 @@ struct blk_queue_tag {
 #define BLK_SCSI_MAX_CMDS	(256)
 #define BLK_SCSI_CMD_PER_LONG	(BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
 
+#ifdef CONFIG_BLK_DEV_ZONED
+enum blk_zone_type {
+	BLK_ZONE_TYPE_UNKNOWN,
+	BLK_ZONE_TYPE_CONVENTIONAL,
+	BLK_ZONE_TYPE_SEQWRITE_REQ,
+	BLK_ZONE_TYPE_SEQWRITE_PREF,
+	BLK_ZONE_TYPE_RESERVED,
+};
+
+enum blk_zone_state {
+	BLK_ZONE_UNKNOWN,
+	BLK_ZONE_NO_WP,
+	BLK_ZONE_OPEN,
+	BLK_ZONE_READONLY,
+	BLK_ZONE_OFFLINE,
+	BLK_ZONE_BUSY,
+};
+
+struct blk_zone {
+	struct rb_node node;
+	spinlock_t lock;
+	sector_t start;
+	size_t len;
+	sector_t wp;
+	enum blk_zone_type type;
+	enum blk_zone_state state;
+	void *private_data;
+};
+
+#define blk_zone_is_smr(z) ((z)->type == BLK_ZONE_TYPE_SEQWRITE_REQ ||	\
+			    (z)->type == BLK_ZONE_TYPE_SEQWRITE_PREF)
+
+#define blk_zone_is_cmr(z) ((z)->type == BLK_ZONE_TYPE_CONVENTIONAL)
+#define blk_zone_is_full(z) ((z)->wp == (z)->start + (z)->len)
+#define blk_zone_is_empty(z) ((z)->wp == (z)->start)
+
+extern struct blk_zone *blk_lookup_zone(struct request_queue *, sector_t);
+extern struct blk_zone *blk_insert_zone(struct request_queue *,
+					struct blk_zone *);
+extern void blk_drop_zones(struct request_queue *);
+#else
+static inline void blk_drop_zones(struct request_queue *q) { };
+#endif
+
 struct queue_limits {
 	unsigned long		bounce_pfn;
 	unsigned long		seg_boundary_mask;
@@ -421,6 +465,9 @@ struct request_queue {
 
 	struct queue_limits	limits;
 
+#ifdef CONFIG_BLK_DEV_ZONED
+	struct rb_root		zones;
+#endif
 	/*
 	 * sg stuff
 	 */
-- 
1.8.5.6

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

[Hannes Reinecke]

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 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 73200b8..7ffbc44 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -229,6 +229,42 @@ 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++;
+	}
+	offset += sprintf(page + offset, "%zu\n", num);
+	return offset;
+}
+#endif
+
 #define QUEUE_SYSFS_BIT_FNS(name, flag, neg)				\
 static ssize_t								\
 queue_show_##name(struct request_queue *q, char *page)			\
@@ -484,6 +520,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
+
 static struct queue_sysfs_entry queue_nonrot_entry = {
 	.attr = {.name = "rotational", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_show_nonrot,
@@ -547,6 +590,9 @@ static struct attribute *default_attrs[] = {
 	&queue_discard_zeroes_data_entry.attr,
 	&queue_write_same_max_entry.attr,
 	&queue_nonrot_entry.attr,
+#ifdef CONFIG_BLK_DEV_ZONED
+	&queue_zoned_entry.attr,
+#endif
 	&queue_nomerges_entry.attr,
 	&queue_rq_affinity_entry.attr,
 	&queue_iostats_entry.attr,
-- 
1.8.5.6

[PATCH 5/6] block: Introduce BLKPREP_DONE

[Hannes Reinecke]

Add a new blkprep return code BLKPREP_DONE to signal completion
without I/O error.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c        | 6 +++++-
 drivers/scsi/scsi_lib.c | 1 +
 include/linux/blkdev.h  | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index e273194..4bcf30a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2462,9 +2462,13 @@ struct request *blk_peek_request(struct request_queue *q)
 
 			rq = NULL;
 			break;
-		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
+		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID ||
+			   ret == BLKPREP_DONE) {
 			int err = (ret == BLKPREP_INVALID) ? -EREMOTEIO : -EIO;
 
+			if (ret == BLKPREP_DONE)
+				err = 0;
+
 			rq->cmd_flags |= REQ_QUIET;
 			/*
 			 * Mark this request as started so we don't trigger
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b2e332a..f112926 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1257,6 +1257,7 @@ scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 	case BLKPREP_KILL:
 	case BLKPREP_INVALID:
 		req->errors = DID_NO_CONNECT << 16;
+	case BLKPREP_DONE:
 		/* release the command and kill it */
 		if (req->special) {
 			struct scsi_cmnd *cmd = req->special;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d5e3d8b..c351444 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -736,6 +736,7 @@ enum {
 	BLKPREP_KILL,		/* fatal error, kill, return -EIO */
 	BLKPREP_DEFER,		/* leave on queue */
 	BLKPREP_INVALID,	/* invalid command, kill, return -EREMOTEIO */
+	BLKPREP_DONE,		/* complete w/o error */
 };
 
 extern unsigned long blk_max_low_pfn, blk_max_pfn;
-- 
1.8.5.6

[PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Hannes Reinecke]

Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
without error.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 block/blk-mq.c          | 1 +
 drivers/scsi/scsi_lib.c | 3 +++
 include/linux/blk-mq.h  | 1 +
 3 files changed, 5 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7df9c92..cd25d68 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -793,6 +793,7 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 			pr_err("blk-mq: bad return on queue: %d\n", ret);
 		case BLK_MQ_RQ_QUEUE_ERROR:
 			rq->errors = -EIO;
+		case BLK_MQ_RQ_QUEUE_DONE:
 			blk_mq_end_request(rq, rq->errors);
 			break;
 		}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f112926..d74108b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1802,6 +1802,8 @@ static inline int prep_to_mq(int ret)
 		return 0;
 	case BLKPREP_DEFER:
 		return BLK_MQ_RQ_QUEUE_BUSY;
+	case BLKPREP_DONE:
+		return BLK_MQ_RQ_QUEUE_DONE;
 	default:
 		return BLK_MQ_RQ_QUEUE_ERROR;
 	}
@@ -1945,6 +1947,7 @@ out:
 			blk_mq_delay_queue(hctx, SCSI_QUEUE_DELAY);
 		break;
 	case BLK_MQ_RQ_QUEUE_ERROR:
+	case BLK_MQ_RQ_QUEUE_DONE:
 		/*
 		 * Make sure to release all allocated ressources when
 		 * we hit an error, as we will never see this command
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 2498fdf..f7a9e97 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -151,6 +151,7 @@ enum {
 	BLK_MQ_RQ_QUEUE_OK	= 0,	/* queued fine */
 	BLK_MQ_RQ_QUEUE_BUSY	= 1,	/* requeue IO for later */
 	BLK_MQ_RQ_QUEUE_ERROR	= 2,	/* end IO with error */
+	BLK_MQ_RQ_QUEUE_DONE	= 3,	/* end IO w/o error */
 
 	BLK_MQ_F_SHOULD_MERGE	= 1 << 0,
 	BLK_MQ_F_TAG_SHARED	= 1 << 1,
-- 
1.8.5.6

Re: [PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> The queue limits already have a 'chunk_sectors' setting, so
> we should be presenting it via sysfs.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/blk-sysfs.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> The queue limits already have a 'chunk_sectors' setting, so
> we should be presenting it via sysfs.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/blk-sysfs.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 2/6] block: update chunk_sectors in blk_stack_limits()

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  block/blk-settings.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 2/6] block: update chunk_sectors in blk_stack_limits()

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  block/blk-settings.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 3/6] block: Implement support for zoned block devices

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Implement a RB-Tree holding the zone information and
> add support functions for maintaining the RB-Tree.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/Kconfig          |  9 +++++++
>  block/Makefile         |  1 +
>  block/blk-core.c       |  5 ++++
>  block/blk-zoned.c      | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/blkdev.h | 47 +++++++++++++++++++++++++++++++++
>  5 files changed, 132 insertions(+)
>  create mode 100644 block/blk-zoned.c

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 3/6] block: Implement support for zoned block devices

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Implement a RB-Tree holding the zone information and
> add support functions for maintaining the RB-Tree.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/Kconfig          |  9 +++++++
>  block/Makefile         |  1 +
>  block/blk-core.c       |  5 ++++
>  block/blk-zoned.c      | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/blkdev.h | 47 +++++++++++++++++++++++++++++++++
>  5 files changed, 132 insertions(+)
>  create mode 100644 block/blk-zoned.c

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

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

[Damien Le Moal]

On 7/19/16 22:20, 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 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

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

[Damien Le Moal]

On 7/19/16 22:20, 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 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 5/6] block: Introduce BLKPREP_DONE

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Add a new blkprep return code BLKPREP_DONE to signal completion
> without I/O error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/blk-core.c        | 6 +++++-
>  drivers/scsi/scsi_lib.c | 1 +
>  include/linux/blkdev.h  | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 5/6] block: Introduce BLKPREP_DONE

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Add a new blkprep return code BLKPREP_DONE to signal completion
> without I/O error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/blk-core.c        | 6 +++++-
>  drivers/scsi/scsi_lib.c | 1 +
>  include/linux/blkdev.h  | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
> without error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  block/blk-mq.c          | 1 +
>  drivers/scsi/scsi_lib.c | 3 +++
>  include/linux/blk-mq.h  | 1 +
>  3 files changed, 5 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Damien Le Moal]

On 7/19/16 22:20, Hannes Reinecke wrote:
> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
> without error.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  block/blk-mq.c          | 1 +
>  drivers/scsi/scsi_lib.c | 3 +++
>  include/linux/blk-mq.h  | 1 +
>  3 files changed, 5 insertions(+)

Reviewed-by: Damien Le Moal <damien.lemoal@hgst.com>
Tested-by: Damien Le Moal <damien.lemoal@hgst.com>

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
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.

Re: [PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 03:20:39PM +0200, Hannes Reinecke wrote:
> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
> without error.

NAK.  You can just do a blk_mq_end_request on the request and return
0 from ->queue_rq.

Re: [PATCH 5/6] block: Introduce BLKPREP_DONE

[Christoph Hellwig]

> +++ b/block/blk-core.c
> @@ -2462,9 +2462,13 @@ struct request *blk_peek_request(struct request_queue *q)
>  
>  			rq = NULL;
>  			break;
> -		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
> +		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID ||
> +			   ret == BLKPREP_DONE) {
>  			int err = (ret == BLKPREP_INVALID) ? -EREMOTEIO : -EIO;
>  
> +			if (ret == BLKPREP_DONE)
> +				err = 0;
> +

Please just use a proper switch statement for the return values.

static void blk_prep_end_request(struct request *req, int error)
{
	rq->cmd_flags |= REQ_QUIET;
	blk_start_request(rq);
	__blk_end_request_all(rq, error);
}

struct request *blk_peek_request(struct request_queue *q)
{

	...


	switch (ret) {
	...
	case BLKPREP_KILL:
		blk_prep_end_request(rq, -EIO);
		break;
	case BLKPREP_INVALID:
		blk_prep_end_request(rq, -EREMOTEIO);
		break;
	case BLKPREP_DONE:
		blk_prep_end_request(rq, 0);
		break;
	...
}

Re: [PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 03:20:34PM +0200, Hannes Reinecke wrote:
> The queue limits already have a 'chunk_sectors' setting, so
> we should be presenting it via sysfs.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Looks fine for 4.8 independent of any ZBC implications:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] block: update chunk_sectors in blk_stack_limits()

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Hannes Reinecke]

On 07/21/2016 07:53 AM, Christoph Hellwig wrote:
> On Tue, Jul 19, 2016 at 03:20:39PM +0200, Hannes Reinecke wrote:
>> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
>> without error.
> 
> NAK.  You can just do a blk_mq_end_request on the request and return
> 0 from ->queue_rq.
> 
Okay, will be doing so.

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)

Re: [PATCH 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Hannes Reinecke]

On 07/21/2016 07:53 AM, Christoph Hellwig wrote:
> On Tue, Jul 19, 2016 at 03:20:39PM +0200, Hannes Reinecke wrote:
>> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
>> without error.
> 
> NAK.  You can just do a blk_mq_end_request on the request and return
> 0 from ->queue_rq.
> 
Okay, will be doing so.

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-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] block: Introduce BLKPREP_DONE

[Hannes Reinecke]

On 07/21/2016 07:58 AM, Christoph Hellwig wrote:
>> +++ b/block/blk-core.c
>> @@ -2462,9 +2462,13 @@ struct request *blk_peek_request(struct request_queue *q)
>>  
>>  			rq = NULL;
>>  			break;
>> -		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
>> +		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID ||
>> +			   ret == BLKPREP_DONE) {
>>  			int err = (ret == BLKPREP_INVALID) ? -EREMOTEIO : -EIO;
>>  
>> +			if (ret == BLKPREP_DONE)
>> +				err = 0;
>> +
> 
> Please just use a proper switch statement for the return values.
> 
Ok.

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)

Re: [PATCH 5/6] block: Introduce BLKPREP_DONE

[Hannes Reinecke]

On 07/21/2016 07:58 AM, Christoph Hellwig wrote:
>> +++ b/block/blk-core.c
>> @@ -2462,9 +2462,13 @@ struct request *blk_peek_request(struct request_queue *q)
>>  
>>  			rq = NULL;
>>  			break;
>> -		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
>> +		} else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID ||
>> +			   ret == BLKPREP_DONE) {
>>  			int err = (ret == BLKPREP_INVALID) ? -EREMOTEIO : -EIO;
>>  
>> +			if (ret == BLKPREP_DONE)
>> +				err = 0;
>> +
> 
> Please just use a proper switch statement for the return values.
> 
Ok.

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

Re: [PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> The queue limits already have a 'chunk_sectors' setting, so we
Hannes> should be presenting it via sysfs.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
Hannes> for zoned devices.

Not quite one value per file :(

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 2/6] block: update chunk_sectors in blk_stack_limits()

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Hannes Reinecke]

On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
> Hannes> for zoned devices.
> 
> Not quite one value per file :(
> 
Yes.
But I wanted to display the zone layout in a concise way allowing
user-space programs to determine the zone layout without having to
issue a 'REPORT ZONES' command themselves.
I found it slightly pointless to add one sysfs entry per zone,
and at the same time a simple 'zone_size' attribute wouldn't cover all
possibilities.

However, as SMR drives seem to stabilise around having a fixed zone size
(with a possible exemption of the last zone to cover left-overs)
I'd be fine a replace this with a single 'zone_size' attribute which
could be set to eg '-1' for drives which indeed would implement variable
zone sizes.

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)

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

[Hannes Reinecke]

On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
> Hannes> for zoned devices.
> 
> Not quite one value per file :(
> 
Yes.
But I wanted to display the zone layout in a concise way allowing
user-space programs to determine the zone layout without having to
issue a 'REPORT ZONES' command themselves.
I found it slightly pointless to add one sysfs entry per zone,
and at the same time a simple 'zone_size' attribute wouldn't cover all
possibilities.

However, as SMR drives seem to stabilise around having a fixed zone size
(with a possible exemption of the last zone to cover left-overs)
I'd be fine a replace this with a single 'zone_size' attribute which
could be set to eg '-1' for drives which indeed would implement variable
zone sizes.

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-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bart Van Assche]

On 07/23/16 05:43, Hannes Reinecke wrote:
> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>
>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>> Hannes> for zoned devices.
>>
>> Not quite one value per file :(
>>
> Yes.
> But I wanted to display the zone layout in a concise way allowing
> user-space programs to determine the zone layout without having to
> issue a 'REPORT ZONES' command themselves.
> I found it slightly pointless to add one sysfs entry per zone,
> and at the same time a simple 'zone_size' attribute wouldn't cover all
> possibilities.
>
> However, as SMR drives seem to stabilise around having a fixed zone size
> (with a possible exemption of the last zone to cover left-overs)
> I'd be fine a replace this with a single 'zone_size' attribute which
> could be set to eg '-1' for drives which indeed would implement variable
> zone sizes.

It's not that hard to convert the information exported by 
queue_zoned_show() from a single sysfs attribute into one directory per 
zone. Doing so would make it much easier for scripts to parse that 
information and would also avoid that the zone information has to be 
truncated because not all of it fits into a single page.

Bart.

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

[Bart Van Assche]

On 07/23/16 05:43, Hannes Reinecke wrote:
> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>
>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>> Hannes> for zoned devices.
>>
>> Not quite one value per file :(
>>
> Yes.
> But I wanted to display the zone layout in a concise way allowing
> user-space programs to determine the zone layout without having to
> issue a 'REPORT ZONES' command themselves.
> I found it slightly pointless to add one sysfs entry per zone,
> and at the same time a simple 'zone_size' attribute wouldn't cover all
> possibilities.
>
> However, as SMR drives seem to stabilise around having a fixed zone size
> (with a possible exemption of the last zone to cover left-overs)
> I'd be fine a replace this with a single 'zone_size' attribute which
> could be set to eg '-1' for drives which indeed would implement variable
> zone sizes.

It's not that hard to convert the information exported by 
queue_zoned_show() from a single sysfs attribute into one directory per 
zone. Doing so would make it much easier for scripts to parse that 
information and would also avoid that the zone information has to be 
truncated because not all of it fits into a single page.

Bart.

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

[Hannes Reinecke]

On 07/24/2016 12:13 AM, Bart Van Assche wrote:
> On 07/23/16 05:43, Hannes Reinecke wrote:
>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>
>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>> Hannes> for zoned devices.
>>>
>>> Not quite one value per file :(
>>>
>> Yes.
>> But I wanted to display the zone layout in a concise way allowing
>> user-space programs to determine the zone layout without having to
>> issue a 'REPORT ZONES' command themselves.
>> I found it slightly pointless to add one sysfs entry per zone,
>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>> possibilities.
>>
>> However, as SMR drives seem to stabilise around having a fixed zone size
>> (with a possible exemption of the last zone to cover left-overs)
>> I'd be fine a replace this with a single 'zone_size' attribute which
>> could be set to eg '-1' for drives which indeed would implement variable
>> zone sizes.
> 
> It's not that hard to convert the information exported by
> queue_zoned_show() from a single sysfs attribute into one directory per
> zone. Doing so would make it much easier for scripts to parse that
> information and would also avoid that the zone information has to be
> truncated because not all of it fits into a single page.
> 
But this is precisely what I've tried to avoid.
Creating one file or directory per zone would mean we'll end up with
rough 20k files/directories.
Which I found rather excessive.

Of course, it that is not a concern that I can easily convert it.

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)

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

[Hannes Reinecke]

On 07/24/2016 12:13 AM, Bart Van Assche wrote:
> On 07/23/16 05:43, Hannes Reinecke wrote:
>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>
>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>> Hannes> for zoned devices.
>>>
>>> Not quite one value per file :(
>>>
>> Yes.
>> But I wanted to display the zone layout in a concise way allowing
>> user-space programs to determine the zone layout without having to
>> issue a 'REPORT ZONES' command themselves.
>> I found it slightly pointless to add one sysfs entry per zone,
>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>> possibilities.
>>
>> However, as SMR drives seem to stabilise around having a fixed zone size
>> (with a possible exemption of the last zone to cover left-overs)
>> I'd be fine a replace this with a single 'zone_size' attribute which
>> could be set to eg '-1' for drives which indeed would implement variable
>> zone sizes.
> 
> It's not that hard to convert the information exported by
> queue_zoned_show() from a single sysfs attribute into one directory per
> zone. Doing so would make it much easier for scripts to parse that
> information and would also avoid that the zone information has to be
> truncated because not all of it fits into a single page.
> 
But this is precisely what I've tried to avoid.
Creating one file or directory per zone would mean we'll end up with
rough 20k files/directories.
Which I found rather excessive.

Of course, it that is not a concern that I can easily convert it.

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-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bart Van Assche]

On 07/24/16 00:10, Hannes Reinecke wrote:
> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>
>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>>> Hannes> for zoned devices.
>>>>
>>>> Not quite one value per file :(
>>>>
>>> Yes.
>>> But I wanted to display the zone layout in a concise way allowing
>>> user-space programs to determine the zone layout without having to
>>> issue a 'REPORT ZONES' command themselves.
>>> I found it slightly pointless to add one sysfs entry per zone,
>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>> possibilities.
>>>
>>> However, as SMR drives seem to stabilise around having a fixed zone size
>>> (with a possible exemption of the last zone to cover left-overs)
>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>> could be set to eg '-1' for drives which indeed would implement variable
>>> zone sizes.
>>
>> It's not that hard to convert the information exported by
>> queue_zoned_show() from a single sysfs attribute into one directory per
>> zone. Doing so would make it much easier for scripts to parse that
>> information and would also avoid that the zone information has to be
>> truncated because not all of it fits into a single page.
>>
> But this is precisely what I've tried to avoid.
> Creating one file or directory per zone would mean we'll end up with
> rough 20k files/directories.
> Which I found rather excessive.
>
> Of course, it that is not a concern that I can easily convert it.

If there are 10K zones and since queue_zoned_show() is limited to one 
page then only a very small fraction of the zone information will be 
available through sysfs. I remember from your presentations that reading 
the zone information is slow. Is 10K zones a typical value or a worst 
case value?

Bart.

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

[Bart Van Assche]

On 07/24/16 00:10, Hannes Reinecke wrote:
> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>
>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>>> Hannes> for zoned devices.
>>>>
>>>> Not quite one value per file :(
>>>>
>>> Yes.
>>> But I wanted to display the zone layout in a concise way allowing
>>> user-space programs to determine the zone layout without having to
>>> issue a 'REPORT ZONES' command themselves.
>>> I found it slightly pointless to add one sysfs entry per zone,
>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>> possibilities.
>>>
>>> However, as SMR drives seem to stabilise around having a fixed zone size
>>> (with a possible exemption of the last zone to cover left-overs)
>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>> could be set to eg '-1' for drives which indeed would implement variable
>>> zone sizes.
>>
>> It's not that hard to convert the information exported by
>> queue_zoned_show() from a single sysfs attribute into one directory per
>> zone. Doing so would make it much easier for scripts to parse that
>> information and would also avoid that the zone information has to be
>> truncated because not all of it fits into a single page.
>>
> But this is precisely what I've tried to avoid.
> Creating one file or directory per zone would mean we'll end up with
> rough 20k files/directories.
> Which I found rather excessive.
>
> Of course, it that is not a concern that I can easily convert it.

If there are 10K zones and since queue_zoned_show() is limited to one 
page then only a very small fraction of the zone information will be 
available through sysfs. I remember from your presentations that reading 
the zone information is slow. Is 10K zones a typical value or a worst 
case value?

Bart.

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

[Bart Van Assche]

On 07/24/16 06:37, Bart Van Assche wrote:
> On 07/24/16 00:10, Hannes Reinecke wrote:
>> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>>
>>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>>>> Hannes> for zoned devices.
>>>>>
>>>>> Not quite one value per file :(
>>>>>
>>>> Yes.
>>>> But I wanted to display the zone layout in a concise way allowing
>>>> user-space programs to determine the zone layout without having to
>>>> issue a 'REPORT ZONES' command themselves.
>>>> I found it slightly pointless to add one sysfs entry per zone,
>>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>>> possibilities.
>>>>
>>>> However, as SMR drives seem to stabilise around having a fixed zone size
>>>> (with a possible exemption of the last zone to cover left-overs)
>>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>>> could be set to eg '-1' for drives which indeed would implement variable
>>>> zone sizes.
>>>
>>> It's not that hard to convert the information exported by
>>> queue_zoned_show() from a single sysfs attribute into one directory per
>>> zone. Doing so would make it much easier for scripts to parse that
>>> information and would also avoid that the zone information has to be
>>> truncated because not all of it fits into a single page.
>>>
>> But this is precisely what I've tried to avoid.
>> Creating one file or directory per zone would mean we'll end up with
>> rough 20k files/directories.
>> Which I found rather excessive.
>>
>> Of course, it that is not a concern that I can easily convert it.
>
> If there are 10K zones and since queue_zoned_show() is limited to one
> page then only a very small fraction of the zone information will be
> available through sysfs. I remember from your presentations that reading
> the zone information is slow. Is 10K zones a typical value or a worst
> case value?

(replying to my own e-mail)

Something I should have asked before: is the zone information intended 
for end users or rather for software developers? In the latter case, 
have you considered to use debugfs instead of sysfs to export this 
information? From Documentation/filesystems/debugfs.txt: "Unlike /proc, 
which is only meant for information about a process, or sysfs, which has 
strict one-value-per-file rules, debugfs has no rules at all."

Bart.

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

[Damien Le Moal]

Bart,


> On Jul 24, 2016, at 22:37, Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> 
> If there are 10K zones and since queue_zoned_show() is limited to one page then only a very small fraction of the zone information will be available through sysfs. I remember from your presentations that reading the zone information is slow. Is 10K zones a typical value or a worst case value?

Our 10 TB drive with 256 MB zones has 37256 zones. A drive with the same capacity
and 128 MB zones will double that (there are some out there with 128 MB zones).
And higher capacities are coming. So 100K+ zones is not unrealistic in the very
near future.

I think that what Hannes proposed, i.e. to have the zoned file only specify the
zone size (so only one value), is OK as long as we also add an ioctl to get the
detailed zone information, which would be much faster for applications than using
something like libzbc which will be sending SG_IOs to retrieve the zone information
from the disk.

Best regards.

------------------------
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
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.

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

[Damien Le Moal]

Bart,


> On Jul 24, 2016, at 22:51, Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> 
> Something I should have asked before: is the zone information intended for end users or rather for software developers? In the latter case, have you considered to use debugfs instead of sysfs to export this information? From Documentation/filesystems/debugfs.txt: "Unlike /proc, which is only meant for information about a process, or sysfs, which has strict one-value-per-file rules, debugfs has no rules at all."

I think that the zone information will definitively be of higher value
for software developers rather than end users. But that information must
be available at run time for things like mkfs.xxx and having to mount
debugfs on a production system to be able to run mkfs on an SMR disk
(or initialize an application using the raw block device) does not sound
ideal.

Back to my previous reply, having the sysfs zoned file specifying just the
disk zone size and having an ioctl to retrieve the detailed zone information
looks to me much more sensible.

Best regards. 

------------------------
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital brand
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
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.

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

[Hannes Reinecke]

On 07/24/2016 03:51 PM, Bart Van Assche wrote:
> On 07/24/16 06:37, Bart Van Assche wrote:
>> On 07/24/16 00:10, Hannes Reinecke wrote:
>>> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>>>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>>>
>>>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone
>>>>>> layout
>>>>>> Hannes> for zoned devices.
>>>>>>
>>>>>> Not quite one value per file :(
>>>>>>
>>>>> Yes.
>>>>> But I wanted to display the zone layout in a concise way allowing
>>>>> user-space programs to determine the zone layout without having to
>>>>> issue a 'REPORT ZONES' command themselves.
>>>>> I found it slightly pointless to add one sysfs entry per zone,
>>>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>>>> possibilities.
>>>>>
>>>>> However, as SMR drives seem to stabilise around having a fixed zone
>>>>> size
>>>>> (with a possible exemption of the last zone to cover left-overs)
>>>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>>>> could be set to eg '-1' for drives which indeed would implement
>>>>> variable
>>>>> zone sizes.
>>>>
>>>> It's not that hard to convert the information exported by
>>>> queue_zoned_show() from a single sysfs attribute into one directory per
>>>> zone. Doing so would make it much easier for scripts to parse that
>>>> information and would also avoid that the zone information has to be
>>>> truncated because not all of it fits into a single page.
>>>>
>>> But this is precisely what I've tried to avoid.
>>> Creating one file or directory per zone would mean we'll end up with
>>> rough 20k files/directories.
>>> Which I found rather excessive.
>>>
>>> Of course, it that is not a concern that I can easily convert it.
>>
>> If there are 10K zones and since queue_zoned_show() is limited to one
>> page then only a very small fraction of the zone information will be
>> available through sysfs. I remember from your presentations that reading
>> the zone information is slow. Is 10K zones a typical value or a worst
>> case value?
> 
> (replying to my own e-mail)
> 
> Something I should have asked before: is the zone information intended
> for end users or rather for software developers? In the latter case,
> have you considered to use debugfs instead of sysfs to export this
> information? From Documentation/filesystems/debugfs.txt: "Unlike /proc,
> which is only meant for information about a process, or sysfs, which has
> strict one-value-per-file rules, debugfs has no rules at all."
> 
I would be perfectly fine with exporting the zone information via debugfs.
But at the same time I would like to have a simple sysfs representation
for SMR drives which can be utilized by mkfs and friend.
Typically SMR drives have a fixed zone size, and more often than not the
size of the zones is identical.
So mkfs would benefit from knowing the fixed zone layout as it then can
arrange the filesystem structures to align to those zones.
At the same time it doesn't need to know the write pointer, so full zone
information is not required here.
And during normal operation ideally the zone information is handled
within the kernel, so again userspace doesn't necessarily need to
have access to the full zone information.

So I guess I'll redo this one to export a sysfs attribute 'zone_size'
(if the zones are of identical size), and make the full zone information
available via debugfs.

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)

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

[Hannes Reinecke]

On 07/24/2016 03:51 PM, Bart Van Assche wrote:
> On 07/24/16 06:37, Bart Van Assche wrote:
>> On 07/24/16 00:10, Hannes Reinecke wrote:
>>> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>>>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>>>
>>>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone
>>>>>> layout
>>>>>> Hannes> for zoned devices.
>>>>>>
>>>>>> Not quite one value per file :(
>>>>>>
>>>>> Yes.
>>>>> But I wanted to display the zone layout in a concise way allowing
>>>>> user-space programs to determine the zone layout without having to
>>>>> issue a 'REPORT ZONES' command themselves.
>>>>> I found it slightly pointless to add one sysfs entry per zone,
>>>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>>>> possibilities.
>>>>>
>>>>> However, as SMR drives seem to stabilise around having a fixed zone
>>>>> size
>>>>> (with a possible exemption of the last zone to cover left-overs)
>>>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>>>> could be set to eg '-1' for drives which indeed would implement
>>>>> variable
>>>>> zone sizes.
>>>>
>>>> It's not that hard to convert the information exported by
>>>> queue_zoned_show() from a single sysfs attribute into one directory per
>>>> zone. Doing so would make it much easier for scripts to parse that
>>>> information and would also avoid that the zone information has to be
>>>> truncated because not all of it fits into a single page.
>>>>
>>> But this is precisely what I've tried to avoid.
>>> Creating one file or directory per zone would mean we'll end up with
>>> rough 20k files/directories.
>>> Which I found rather excessive.
>>>
>>> Of course, it that is not a concern that I can easily convert it.
>>
>> If there are 10K zones and since queue_zoned_show() is limited to one
>> page then only a very small fraction of the zone information will be
>> available through sysfs. I remember from your presentations that reading
>> the zone information is slow. Is 10K zones a typical value or a worst
>> case value?
> 
> (replying to my own e-mail)
> 
> Something I should have asked before: is the zone information intended
> for end users or rather for software developers? In the latter case,
> have you considered to use debugfs instead of sysfs to export this
> information? From Documentation/filesystems/debugfs.txt: "Unlike /proc,
> which is only meant for information about a process, or sysfs, which has
> strict one-value-per-file rules, debugfs has no rules at all."
> 
I would be perfectly fine with exporting the zone information via debugfs.
But at the same time I would like to have a simple sysfs representation
for SMR drives which can be utilized by mkfs and friend.
Typically SMR drives have a fixed zone size, and more often than not the
size of the zones is identical.
So mkfs would benefit from knowing the fixed zone layout as it then can
arrange the filesystem structures to align to those zones.
At the same time it doesn't need to know the write pointer, so full zone
information is not required here.
And during normal operation ideally the zone information is handled
within the kernel, so again userspace doesn't necessarily need to
have access to the full zone information.

So I guess I'll redo this one to export a sysfs attribute 'zone_size'
(if the zones are of identical size), and make the full zone information
available via debugfs.

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

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

[Jens Axboe]

On 07/24/2016 07:37 AM, Bart Van Assche wrote:
> On 07/24/16 00:10, Hannes Reinecke wrote:
>> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>>
>>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone layout
>>>>> Hannes> for zoned devices.
>>>>>
>>>>> Not quite one value per file :(
>>>>>
>>>> Yes.
>>>> But I wanted to display the zone layout in a concise way allowing
>>>> user-space programs to determine the zone layout without having to
>>>> issue a 'REPORT ZONES' command themselves.
>>>> I found it slightly pointless to add one sysfs entry per zone,
>>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>>> possibilities.
>>>>
>>>> However, as SMR drives seem to stabilise around having a fixed zone
>>>> size
>>>> (with a possible exemption of the last zone to cover left-overs)
>>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>>> could be set to eg '-1' for drives which indeed would implement
>>>> variable
>>>> zone sizes.
>>>
>>> It's not that hard to convert the information exported by
>>> queue_zoned_show() from a single sysfs attribute into one directory per
>>> zone. Doing so would make it much easier for scripts to parse that
>>> information and would also avoid that the zone information has to be
>>> truncated because not all of it fits into a single page.
>>>
>> But this is precisely what I've tried to avoid.
>> Creating one file or directory per zone would mean we'll end up with
>> rough 20k files/directories.
>> Which I found rather excessive.
>>
>> Of course, it that is not a concern that I can easily convert it.
>
> If there are 10K zones and since queue_zoned_show() is limited to one
> page then only a very small fraction of the zone information will be
> available through sysfs. I remember from your presentations that reading
> the zone information is slow. Is 10K zones a typical value or a worst
> case value?

Just kill the sysfs file, it's useless for many zones. Either have the 
application use some library to provide the information, or use bsg and 
similar to get it.

-- 
Jens Axboe

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

[Jens Axboe]

On 07/24/2016 11:56 PM, Hannes Reinecke wrote:
> On 07/24/2016 03:51 PM, Bart Van Assche wrote:
>> On 07/24/16 06:37, Bart Van Assche wrote:
>>> On 07/24/16 00:10, Hannes Reinecke wrote:
>>>> On 07/24/2016 12:13 AM, Bart Van Assche wrote:
>>>>> On 07/23/16 05:43, Hannes Reinecke wrote:
>>>>>> On 07/22/2016 10:45 PM, Martin K. Petersen wrote:
>>>>>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>>>>>
>>>>>>> Hannes> Add a sysfs queue attribute 'zoned' to display the zone
>>>>>>> layout
>>>>>>> Hannes> for zoned devices.
>>>>>>>
>>>>>>> Not quite one value per file :(
>>>>>>>
>>>>>> Yes.
>>>>>> But I wanted to display the zone layout in a concise way allowing
>>>>>> user-space programs to determine the zone layout without having to
>>>>>> issue a 'REPORT ZONES' command themselves.
>>>>>> I found it slightly pointless to add one sysfs entry per zone,
>>>>>> and at the same time a simple 'zone_size' attribute wouldn't cover all
>>>>>> possibilities.
>>>>>>
>>>>>> However, as SMR drives seem to stabilise around having a fixed zone
>>>>>> size
>>>>>> (with a possible exemption of the last zone to cover left-overs)
>>>>>> I'd be fine a replace this with a single 'zone_size' attribute which
>>>>>> could be set to eg '-1' for drives which indeed would implement
>>>>>> variable
>>>>>> zone sizes.
>>>>>
>>>>> It's not that hard to convert the information exported by
>>>>> queue_zoned_show() from a single sysfs attribute into one directory per
>>>>> zone. Doing so would make it much easier for scripts to parse that
>>>>> information and would also avoid that the zone information has to be
>>>>> truncated because not all of it fits into a single page.
>>>>>
>>>> But this is precisely what I've tried to avoid.
>>>> Creating one file or directory per zone would mean we'll end up with
>>>> rough 20k files/directories.
>>>> Which I found rather excessive.
>>>>
>>>> Of course, it that is not a concern that I can easily convert it.
>>>
>>> If there are 10K zones and since queue_zoned_show() is limited to one
>>> page then only a very small fraction of the zone information will be
>>> available through sysfs. I remember from your presentations that reading
>>> the zone information is slow. Is 10K zones a typical value or a worst
>>> case value?
>>
>> (replying to my own e-mail)
>>
>> Something I should have asked before: is the zone information intended
>> for end users or rather for software developers? In the latter case,
>> have you considered to use debugfs instead of sysfs to export this
>> information? From Documentation/filesystems/debugfs.txt: "Unlike /proc,
>> which is only meant for information about a process, or sysfs, which has
>> strict one-value-per-file rules, debugfs has no rules at all."
>>
> I would be perfectly fine with exporting the zone information via debugfs.
> But at the same time I would like to have a simple sysfs representation
> for SMR drives which can be utilized by mkfs and friend.

Then provide a real interface, through a library. Don't expose tons of 
zones through sysfs, and expect applications to parse that. That's silly.

-- 
Jens Axboe