[PATCH V4 00/16] scsi-mq support for ZBC disks

[PATCH V4 00/16] scsi-mq support for ZBC disks

[Damien Le Moal]

This series implements support for ZBC disks used through the scsi-mq I/O path.

The current scsi level support of ZBC disks guarantees write request ordering
using a per-zone write lock which prevents issuing simultaneously multiple
write commands to a zone, doing so avoid reordering of sequential writes to
sequential zones. This method is however ineffective when scsi-mq is used with
zoned block devices. This is due to the different execution model of blk-mq
which passes a request to the scsi layer for dispatching after the request has
been removed from the I/O scheduler queue. That is, when the scsi layer tries
to lock the target zone of the request, the request may already be out of
order and zone write locking fails to prevent that.

Various approaches have been tried to solve this problem. All of them had the
serious disadvantage of cluttering blk-mq code with zoned block device specific
conditions and processing. As such extensive changes can only turn into a
maintenance nightmares, a radically different solution is proposed here.

This series proposes implementing scsi-mq support for zoned block devices at
the I/O scheduler level with simple modifications of the mq-deadline scheduler.
the modifications are the addition of a per zone write locking mechanism
similar to that implemented in sd_zbc.c for the legacy scsi path. The zone
write locking mechanism is used for the exact same purpose, that is, to limit
writes per zone to at most one request to avoid reordering. The locking context
however changes from that of scsi-sq and is moved to the dispatch_request
method of the scheduler. Within this context, under a spin lock guaranteeing
atomicity against other dispatch contexts, target zones of write requests can
be locked before write requests removal from the scheduler. In effect, this
results in the same behavior as the legacy scsi path. Sequential write ordering
is preserved.

The changes to mq-deadline do not affect regular disks: the same scheduling
behavior is maintained for these. The modification are also optimized to not
lock conventional zones. To do so, additional data is introduced in the
request queue structure so that the low level scsi code can pass upward
information such as the total number of zones and zone types of the device.
The availability of this new data avoids difficulties in accessing this
information from the I/O scheduler initialization method (init_queue() method)
context.

Of note is that patch 15 of this series removes setting mq-deadline as the
default scheduler for block devices with a single hardware queue. The reason for
this is that setting the default scheduler is done very early in the device
initialization sequence, when the disk characteristics are not yet known. This
results in mq-deadline not correctly setting the default zones write locking
behavior nased on the device zoning model. Setting of a default I/O scheduler
can be done easily with udev rules later in the system initialization process,
leading to correct default settings for zoned block devices.

Comments are as always very much appreciated.

Changes from v3:
* Integrated support directly into mq-deadline instead of creating a new I/O
  scheduler.
* Disable setting of default mq scheduler for single queue devices

Changes from v2:
* Introduced blk_zoned structure
* Moved I/O scheduler from drivers/scsi to block 

Changes from v1:
* Addressed Bart's comments for the blk-mq patches (declarations files)
* Split (former) patch 4 into multiple patches to facilitate review
* Fixed scsi disk lookup from io scheduler by introducing
  scsi_disk_from_queue()

Damien Le Moal (12):
  block: Fix declaration of blk-mq debugfs functions
  block: Fix declaration of blk-mq scheduler functions
  block: Add zoned block device information to request queue
  scsi: sd_zbc: Move ZBC declarations to scsi_proto.h
  scsi: sd_zbc: Fix comments and indentation
  scsi: sd_zbc: Rearrange code
  scsi: sd_zbc: Use well defined macros
  scsi: sd_zbc: Fix sd_zbc_read_zoned_characteristics()
  scsi: sd_zbc: Initialize device queue zoned structure
  scsi: sd_zbc: Limit zone write locking to sequential zones
  scsi: sd_zbc: Disable zone write locking with scsi-mq
  block: Introduce zoned I/O scheduler

 Documentation/block/zoned-iosched.txt |  48 ++
 block/Kconfig.iosched                 |  12 +
 block/Makefile                        |   1 +
 block/blk-mq-debugfs.h                |  14 +-
 block/blk-mq-sched.h                  |  11 +-
 block/zoned-iosched.c                 | 925 ++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_lib.c               |   5 +-
 drivers/scsi/sd_zbc.c                 | 267 +++++++---
 include/linux/blk-mq-debugfs.h        |  23 +
 include/linux/blk-mq-sched.h          |  14 +
 include/linux/blkdev.h                |  24 +
 include/scsi/scsi_proto.h             |  45 +-
 12 files changed, 1283 insertions(+), 106 deletions(-)
 create mode 100644 Documentation/block/zoned-iosched.txt
 create mode 100644 block/zoned-iosched.c
 create mode 100644 include/linux/blk-mq-debugfs.h
 create mode 100644 include/linux/blk-mq-sched.h

-- 
2.13.5






*** BLURB HERE ***

Damien Le Moal (16):
  scsi: sd_zbc: Move ZBC declarations to scsi_proto.h
  scsi: sd_zbc: Fix comments and indentation
  scsi: sd_zbc: Rearrange code
  scsi: sd_zbc: Use well defined macros
  scsi: sd_zbc: Fix sd_zbc_read_zoned_characteristics()
  block: Add zoned block device information to request queue
  scsi: sd_zbc: Initialize device request queue zoned data
  scsi: sd_zbc: Limit zone write locking to sequential zones
  scsi: sd_zbc: Disable zone write locking with scsi-mq
  block: mq-deadline: Add zoned block device data
  block: mq-deadline: Introduce zones_wlock attribute
  blokc: mq-deadline: Introduce dispatch helpers
  block: mq-deadline: Introduce zone locking support
  block: mq-deadline: Limit write dispatch for zoned block devices
  block: do not set mq defaulte scheduler
  block: mq-deadline: Update documentation

 Documentation/block/deadline-iosched.txt |  17 ++
 block/elevator.c                         |  17 +-
 block/mq-deadline.c                      | 302 +++++++++++++++++++++++++++-
 drivers/scsi/scsi_lib.c                  |   5 +-
 drivers/scsi/sd_zbc.c                    | 325 ++++++++++++++++++++++++-------
 include/linux/blkdev.h                   |  48 +++++
 include/scsi/scsi_proto.h                |  45 +++--
 7 files changed, 656 insertions(+), 103 deletions(-)

-- 
2.13.5

[PATCH V4 01/16] scsi: sd_zbc: Move ZBC declarations to scsi_proto.h

[Damien Le Moal]

Move standard macro definitions for the zone types and zone conditions
to scsi_proto.h together with the definitions related to the
REPORT ZONES command. While at it, define all values in the enums to
be clear.

Also remove unnecessary includes in sd_zbc.c.

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Bart Van Assche <Bart.VanAssche@wdc.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd_zbc.c     | 24 ------------------------
 include/scsi/scsi_proto.h | 45 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 8aa54779aac1..692c8cbc7ed8 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -28,32 +28,8 @@
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
-#include <scsi/scsi_dbg.h>
-#include <scsi/scsi_device.h>
-#include <scsi/scsi_driver.h>
-#include <scsi/scsi_host.h>
-#include <scsi/scsi_eh.h>
 
 #include "sd.h"
-#include "scsi_priv.h"
-
-enum zbc_zone_type {
-	ZBC_ZONE_TYPE_CONV = 0x1,
-	ZBC_ZONE_TYPE_SEQWRITE_REQ,
-	ZBC_ZONE_TYPE_SEQWRITE_PREF,
-	ZBC_ZONE_TYPE_RESERVED,
-};
-
-enum zbc_zone_cond {
-	ZBC_ZONE_COND_NO_WP,
-	ZBC_ZONE_COND_EMPTY,
-	ZBC_ZONE_COND_IMP_OPEN,
-	ZBC_ZONE_COND_EXP_OPEN,
-	ZBC_ZONE_COND_CLOSED,
-	ZBC_ZONE_COND_READONLY = 0xd,
-	ZBC_ZONE_COND_FULL,
-	ZBC_ZONE_COND_OFFLINE,
-};
 
 /**
  * Convert a zone descriptor to a zone struct.
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index 8c285d9a06d8..39130a9c05bf 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -301,19 +301,42 @@ struct scsi_lun {
 
 /* Reporting options for REPORT ZONES */
 enum zbc_zone_reporting_options {
-	ZBC_ZONE_REPORTING_OPTION_ALL = 0,
-	ZBC_ZONE_REPORTING_OPTION_EMPTY,
-	ZBC_ZONE_REPORTING_OPTION_IMPLICIT_OPEN,
-	ZBC_ZONE_REPORTING_OPTION_EXPLICIT_OPEN,
-	ZBC_ZONE_REPORTING_OPTION_CLOSED,
-	ZBC_ZONE_REPORTING_OPTION_FULL,
-	ZBC_ZONE_REPORTING_OPTION_READONLY,
-	ZBC_ZONE_REPORTING_OPTION_OFFLINE,
-	ZBC_ZONE_REPORTING_OPTION_NEED_RESET_WP = 0x10,
-	ZBC_ZONE_REPORTING_OPTION_NON_SEQWRITE,
-	ZBC_ZONE_REPORTING_OPTION_NON_WP = 0x3f,
+	ZBC_ZONE_REPORTING_OPTION_ALL		= 0x00,
+	ZBC_ZONE_REPORTING_OPTION_EMPTY		= 0x01,
+	ZBC_ZONE_REPORTING_OPTION_IMPLICIT_OPEN	= 0x02,
+	ZBC_ZONE_REPORTING_OPTION_EXPLICIT_OPEN	= 0x03,
+	ZBC_ZONE_REPORTING_OPTION_CLOSED	= 0x04,
+	ZBC_ZONE_REPORTING_OPTION_FULL		= 0x05,
+	ZBC_ZONE_REPORTING_OPTION_READONLY	= 0x06,
+	ZBC_ZONE_REPORTING_OPTION_OFFLINE	= 0x07,
+	/* 0x08 to 0x0f are reserved */
+	ZBC_ZONE_REPORTING_OPTION_NEED_RESET_WP	= 0x10,
+	ZBC_ZONE_REPORTING_OPTION_NON_SEQWRITE	= 0x11,
+	/* 0x12 to 0x3e are reserved */
+	ZBC_ZONE_REPORTING_OPTION_NON_WP	= 0x3f,
 };
 
 #define ZBC_REPORT_ZONE_PARTIAL 0x80
 
+/* Zone types of REPORT ZONES zone descriptors */
+enum zbc_zone_type {
+	ZBC_ZONE_TYPE_CONV		= 0x1,
+	ZBC_ZONE_TYPE_SEQWRITE_REQ	= 0x2,
+	ZBC_ZONE_TYPE_SEQWRITE_PREF	= 0x3,
+	/* 0x4 to 0xf are reserved */
+};
+
+/* Zone conditions of REPORT ZONES zone descriptors */
+enum zbc_zone_cond {
+	ZBC_ZONE_COND_NO_WP		= 0x0,
+	ZBC_ZONE_COND_EMPTY		= 0x1,
+	ZBC_ZONE_COND_IMP_OPEN		= 0x2,
+	ZBC_ZONE_COND_EXP_OPEN		= 0x3,
+	ZBC_ZONE_COND_CLOSED		= 0x4,
+	/* 0x5 to 0xc are reserved */
+	ZBC_ZONE_COND_READONLY		= 0xd,
+	ZBC_ZONE_COND_FULL		= 0xe,
+	ZBC_ZONE_COND_OFFLINE		= 0xf,
+};
+
 #endif /* _SCSI_PROTO_H_ */
-- 
2.13.5

[PATCH V4 02/16] scsi: sd_zbc: Fix comments and indentation

[Damien Le Moal]

Fix comments style (use kernel-doc style) and content to clarify some
functions. Also fix some functions signature indentation and remove a
useless blank line in sd_zbc_read_zones().

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/scsi_lib.c |   5 ++-
 drivers/scsi/sd_zbc.c   | 117 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9cf6a80fe297..c72b97a74906 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1752,7 +1752,10 @@ static void scsi_done(struct scsi_cmnd *cmd)
  *
  * Returns:     Nothing
  *
- * Lock status: IO request lock assumed to be held when called.
+ * Lock status: request queue lock assumed to be held when called.
+ *
+ * Note: See sd_zbc.c sd_zbc_write_lock_zone() for write order
+ * protection for ZBC disks.
  */
 static void scsi_request_fn(struct request_queue *q)
 	__releases(q->queue_lock)
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 692c8cbc7ed8..023f705ae235 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -32,10 +32,14 @@
 #include "sd.h"
 
 /**
- * Convert a zone descriptor to a zone struct.
+ * sd_zbc_parse_report - Convert a zone descriptor to a struct blk_zone,
+ * @sdkp: The disk the report originated from
+ * @buf: Address of the report zone descriptor
+ * @zone: the destination zone structure
+ *
+ * All LBA sized values are converted to 512B sectors unit.
  */
-static void sd_zbc_parse_report(struct scsi_disk *sdkp,
-				u8 *buf,
+static void sd_zbc_parse_report(struct scsi_disk *sdkp, u8 *buf,
 				struct blk_zone *zone)
 {
 	struct scsi_device *sdp = sdkp->device;
@@ -58,7 +62,13 @@ static void sd_zbc_parse_report(struct scsi_disk *sdkp,
 }
 
 /**
- * Issue a REPORT ZONES scsi command.
+ * sd_zbc_report_zones - Issue a REPORT ZONES scsi command.
+ * @sdkp: The target disk
+ * @buf: Buffer to use for the reply
+ * @buflen: the buffer size
+ * @lba: Start LBA of the report
+ *
+ * For internal use during device validation.
  */
 static int sd_zbc_report_zones(struct scsi_disk *sdkp, unsigned char *buf,
 			       unsigned int buflen, sector_t lba)
@@ -99,6 +109,12 @@ static int sd_zbc_report_zones(struct scsi_disk *sdkp, unsigned char *buf,
 	return 0;
 }
 
+/**
+ * sd_zbc_setup_report_cmnd - Prepare a REPORT ZONES scsi command
+ * @cmd: The command to setup
+ *
+ * Call in sd_init_command() for a REQ_OP_ZONE_REPORT request.
+ */
 int sd_zbc_setup_report_cmnd(struct scsi_cmnd *cmd)
 {
 	struct request *rq = cmd->request;
@@ -141,6 +157,14 @@ int sd_zbc_setup_report_cmnd(struct scsi_cmnd *cmd)
 	return BLKPREP_OK;
 }
 
+/**
+ * sd_zbc_report_zones_complete - Process a REPORT ZONES scsi command reply.
+ * @scmd: The completed report zones command
+ * @good_bytes: reply size in bytes
+ *
+ * Convert all reported zone descriptors to struct blk_zone. The conversion
+ * is done in-place, directly in the request specified sg buffer.
+ */
 static void sd_zbc_report_zones_complete(struct scsi_cmnd *scmd,
 					 unsigned int good_bytes)
 {
@@ -196,17 +220,32 @@ static void sd_zbc_report_zones_complete(struct scsi_cmnd *scmd,
 	local_irq_restore(flags);
 }
 
+/**
+ * sd_zbc_zone_sectors - Get the device zone size in number of 512B sectors.
+ * @sdkp: The target disk
+ */
 static inline sector_t sd_zbc_zone_sectors(struct scsi_disk *sdkp)
 {
 	return logical_to_sectors(sdkp->device, sdkp->zone_blocks);
 }
 
+/**
+ * sd_zbc_zone_no - Get the number of the zone conataining a sector.
+ * @sdkp: The target disk
+ * @sector: 512B sector address contained in the zone
+ */
 static inline unsigned int sd_zbc_zone_no(struct scsi_disk *sdkp,
 					  sector_t sector)
 {
 	return sectors_to_logical(sdkp->device, sector) >> sdkp->zone_shift;
 }
 
+/**
+ * sd_zbc_setup_reset_cmnd - Prepare a RESET WRITE POINTER scsi command.
+ * @cmd: the command to setup
+ *
+ * Called from sd_init_command() for a REQ_OP_ZONE_RESET request.
+ */
 int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd)
 {
 	struct request *rq = cmd->request;
@@ -239,6 +278,23 @@ int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd)
 	return BLKPREP_OK;
 }
 
+/**
+ * sd_zbc_write_lock_zone - Write lock a sequential zone.
+ * @cmd: write command
+ *
+ * Called from sd_init_cmd() for write requests (standard write, write same or
+ * write zeroes operations). If the request target zone is not already locked,
+ * the zone is locked and BLKPREP_OK returned, allowing the request to proceed
+ * through dispatch in scsi_request_fn(). Otherwise, BLKPREP_DEFER is returned,
+ * forcing the request to wait for the zone to be unlocked, that is, for the
+ * previously issued write request targeting the same zone to complete.
+ *
+ * This is called from blk_peek_request() context with the queue lock held and
+ * before the request is removed from the scheduler. As a result, multiple
+ * contexts executing concurrently scsi_request_fn() cannot result in write
+ * sequence reordering as only a single write request per zone is allowed to
+ * proceed.
+ */
 int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 {
 	struct request *rq = cmd->request;
@@ -261,10 +317,7 @@ int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 	 * Do not issue more than one write at a time per
 	 * zone. This solves write ordering problems due to
 	 * the unlocking of the request queue in the dispatch
-	 * path in the non scsi-mq case. For scsi-mq, this
-	 * also avoids potential write reordering when multiple
-	 * threads running on different CPUs write to the same
-	 * zone (with a synchronized sequential pattern).
+	 * path in the non scsi-mq case.
 	 */
 	if (sdkp->zones_wlock &&
 	    test_and_set_bit(zno, sdkp->zones_wlock))
@@ -276,6 +329,13 @@ int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 	return BLKPREP_OK;
 }
 
+/**
+ * sd_zbc_write_unlock_zone - Write unlock a sequential zone.
+ * @cmd: write command
+ *
+ * Called from sd_uninit_cmd(). Unlocking the request target zone will allow
+ * dispatching the next write request for the zone.
+ */
 void sd_zbc_write_unlock_zone(struct scsi_cmnd *cmd)
 {
 	struct request *rq = cmd->request;
@@ -290,8 +350,16 @@ void sd_zbc_write_unlock_zone(struct scsi_cmnd *cmd)
 	}
 }
 
-void sd_zbc_complete(struct scsi_cmnd *cmd,
-		     unsigned int good_bytes,
+/**
+ * sd_zbc_complete - ZBC command post processing.
+ * @cmd: Completed command
+ * @good_bytes: Command reply bytes
+ * @sshdr: command sense header
+ *
+ * Called from sd_done(). Process report zones reply and handle reset zone
+ * and write commands errors.
+ */
+void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes,
 		     struct scsi_sense_hdr *sshdr)
 {
 	int result = cmd->result;
@@ -336,7 +404,11 @@ void sd_zbc_complete(struct scsi_cmnd *cmd,
 }
 
 /**
- * Read zoned block device characteristics (VPD page B6).
+ * sd_zbc_read_zoned_characteristics - Read zoned block device characteristics
+ * @sdkp: Target disk
+ * @buf: Buffer where to store the VPD page data
+ *
+ * Read VPD page B6.
  */
 static int sd_zbc_read_zoned_characteristics(struct scsi_disk *sdkp,
 					     unsigned char *buf)
@@ -366,10 +438,16 @@ static int sd_zbc_read_zoned_characteristics(struct scsi_disk *sdkp,
 }
 
 /**
- * Check reported capacity.
+ * sd_zbc_check_capacity - Check reported capacity.
+ * @sdkp: Target disk
+ * @buf: Buffer to use for commands
+ *
+ * ZBC drive may report only the capacity of the first conventional zones at
+ * LBA 0. This is indicated by the RC_BASIS field of the read capacity reply.
+ * Check this here. If the disk reported only its conventional zones capacity,
+ * get the total capacity by doing a report zones.
  */
-static int sd_zbc_check_capacity(struct scsi_disk *sdkp,
-				 unsigned char *buf)
+static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf)
 {
 	sector_t lba;
 	int ret;
@@ -399,6 +477,13 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp,
 
 #define SD_ZBC_BUF_SIZE 131072
 
+/**
+ * sd_zbc_check_zone_size - Check the device zone sizes
+ * @sdkp: Target disk
+ *
+ * Check that all zones of the device are equal. The last zone can however
+ * be smaller. The zone size must also be a power of two number of LBAs.
+ */
 static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
 {
 	u64 zone_blocks;
@@ -525,8 +610,7 @@ static int sd_zbc_setup(struct scsi_disk *sdkp)
 	return 0;
 }
 
-int sd_zbc_read_zones(struct scsi_disk *sdkp,
-		      unsigned char *buf)
+int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
 {
 	int ret;
 
@@ -537,7 +621,6 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp,
 		 */
 		return 0;
 
-
 	/* Get zoned block device characteristics */
 	ret = sd_zbc_read_zoned_characteristics(sdkp, buf);
 	if (ret)
-- 
2.13.5

[PATCH V4 03/16] scsi: sd_zbc: Rearrange code

[Damien Le Moal]

Rearrange sd_zbc_setup() to include use_16_for_rw and use_10_for_rw
assignments and move the calculation of sdkp->zone_shift together
with the assignment of the verified zone_blocks value in
sd_zbc_check_zone_size().

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd_zbc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 023f705ae235..7dbaf920679e 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -584,6 +584,7 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
 	}
 
 	sdkp->zone_blocks = zone_blocks;
+	sdkp->zone_shift = ilog2(zone_blocks);
 
 	return 0;
 }
@@ -591,10 +592,13 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
 static int sd_zbc_setup(struct scsi_disk *sdkp)
 {
 
+	/* READ16/WRITE16 is mandatory for ZBC disks */
+	sdkp->device->use_16_for_rw = 1;
+	sdkp->device->use_10_for_rw = 0;
+
 	/* chunk_sectors indicates the zone size */
 	blk_queue_chunk_sectors(sdkp->disk->queue,
 			logical_to_sectors(sdkp->device, sdkp->zone_blocks));
-	sdkp->zone_shift = ilog2(sdkp->zone_blocks);
 	sdkp->nr_zones = sdkp->capacity >> sdkp->zone_shift;
 	if (sdkp->capacity & (sdkp->zone_blocks - 1))
 		sdkp->nr_zones++;
@@ -657,10 +661,6 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
 	if (ret)
 		goto err;
 
-	/* READ16/WRITE16 is mandatory for ZBC disks */
-	sdkp->device->use_16_for_rw = 1;
-	sdkp->device->use_10_for_rw = 0;
-
 	return 0;
 
 err:
-- 
2.13.5

[PATCH V4 04/16] scsi: sd_zbc: Use well defined macros

[Damien Le Moal]

instead of open coding, use the min() macro to calculate a report zones
reply buffer length in sd_zbc_check_zone_size() and the round_up()
macro for calculating the number of zones in sd_zbc_setup().

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd_zbc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 7dbaf920679e..bbad851c1789 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -475,7 +475,7 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf)
 	return 0;
 }
 
-#define SD_ZBC_BUF_SIZE 131072
+#define SD_ZBC_BUF_SIZE 131072U
 
 /**
  * sd_zbc_check_zone_size - Check the device zone sizes
@@ -526,10 +526,7 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
 		/* Parse REPORT ZONES header */
 		list_length = get_unaligned_be32(&buf[0]) + 64;
 		rec = buf + 64;
-		if (list_length < SD_ZBC_BUF_SIZE)
-			buf_len = list_length;
-		else
-			buf_len = SD_ZBC_BUF_SIZE;
+		buf_len = min(list_length, SD_ZBC_BUF_SIZE);
 
 		/* Parse zone descriptors */
 		while (rec < buf + buf_len) {
@@ -599,9 +596,8 @@ static int sd_zbc_setup(struct scsi_disk *sdkp)
 	/* chunk_sectors indicates the zone size */
 	blk_queue_chunk_sectors(sdkp->disk->queue,
 			logical_to_sectors(sdkp->device, sdkp->zone_blocks));
-	sdkp->nr_zones = sdkp->capacity >> sdkp->zone_shift;
-	if (sdkp->capacity & (sdkp->zone_blocks - 1))
-		sdkp->nr_zones++;
+	sdkp->nr_zones =
+		round_up(sdkp->capacity, sdkp->zone_blocks) >> sdkp->zone_shift;
 
 	if (!sdkp->zones_wlock) {
 		sdkp->zones_wlock = kcalloc(BITS_TO_LONGS(sdkp->nr_zones),
-- 
2.13.5

[PATCH V4 05/16] scsi: sd_zbc: Fix sd_zbc_read_zoned_characteristics()

[Damien Le Moal]

The three values starting at byte 8 of the Zoned Block Device
Characteristics VPD page B6h are 32 bits values, not 64bits. So use
get_unaligned_be32() to retrieve the values and not get_unaligned_be64()

Fixes: 89d947561077 ("sd: Implement support for ZBC devices")
Cc: <stable@vger.kernel.org>

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Bart Van Assche <Bart.VanAssche@wdc.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd_zbc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index bbad851c1789..27793b9f54c0 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -423,15 +423,15 @@ static int sd_zbc_read_zoned_characteristics(struct scsi_disk *sdkp,
 	if (sdkp->device->type != TYPE_ZBC) {
 		/* Host-aware */
 		sdkp->urswrz = 1;
-		sdkp->zones_optimal_open = get_unaligned_be64(&buf[8]);
-		sdkp->zones_optimal_nonseq = get_unaligned_be64(&buf[12]);
+		sdkp->zones_optimal_open = get_unaligned_be32(&buf[8]);
+		sdkp->zones_optimal_nonseq = get_unaligned_be32(&buf[12]);
 		sdkp->zones_max_open = 0;
 	} else {
 		/* Host-managed */
 		sdkp->urswrz = buf[4] & 1;
 		sdkp->zones_optimal_open = 0;
 		sdkp->zones_optimal_nonseq = 0;
-		sdkp->zones_max_open = get_unaligned_be64(&buf[16]);
+		sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
 	}
 
 	return 0;
-- 
2.13.5

[PATCH V4 06/16] block: Add zoned block device information to request queue

[Damien Le Moal]

Components relying only on the requeuest_queue structure for accessing
block devices (e.g. I/O schedulers) have a limited knowledged of the
device characteristics. In particular, the device capacity cannot be
easily discovered, which for a zoned block device also result in the
inability to easily know the number of zones of the device (the zone
size is indicated by the chunk_sectors field of the queue limits).

Introduce the nr_zones field to the request_queue sturcture to simplify
access to this information. Also, add the seq_zones bitmap which
indicates which zones of the device are sequential (write preferred or
write required) zones. These two fields can be initialized by the low
level block device driver (sd.c for ZBC/ZAC disks), and if desired,
by stacking drivers too (device mappers).

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 include/linux/blkdev.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 460294bb0fa5..f53bc4191dd6 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -544,6 +544,13 @@ struct request_queue {
 	struct queue_limits	limits;
 
 	/*
+	 * Zoned block device information for mq I/O schedulers.
+	 * Set by low level device driver (stacking driver may not set this).
+	 */
+	unsigned int	nr_zones;
+	unsigned long	*seq_zones;
+
+	/*
 	 * sg stuff
 	 */
 	unsigned int		sg_timeout;
@@ -785,6 +792,27 @@ static inline unsigned int blk_queue_zone_sectors(struct request_queue *q)
 	return blk_queue_is_zoned(q) ? q->limits.chunk_sectors : 0;
 }
 
+static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
+{
+	return q->nr_zones;
+}
+
+static inline unsigned int blk_queue_zone_no(struct request_queue *q,
+					     sector_t sector)
+{
+	if (!blk_queue_is_zoned(q))
+		return 0;
+	return sector >> ilog2(q->limits.chunk_sectors);
+}
+
+static inline bool blk_queue_zone_is_seq(struct request_queue *q,
+					 sector_t sector)
+{
+	if (!blk_queue_is_zoned(q) || !q->seq_zones)
+		return false;
+	return test_bit(blk_queue_zone_no(q, sector), q->seq_zones);
+}
+
 static inline bool rq_is_sync(struct request *rq)
 {
 	return op_is_sync(rq->cmd_flags);
@@ -1031,6 +1059,16 @@ static inline unsigned int blk_rq_cur_sectors(const struct request *rq)
 	return blk_rq_cur_bytes(rq) >> 9;
 }
 
+static inline unsigned int blk_rq_zone_no(struct request *rq)
+{
+	return blk_queue_zone_no(rq->q, blk_rq_pos(rq));
+}
+
+static inline unsigned int blk_rq_zone_is_seq(struct request *rq)
+{
+	return blk_queue_zone_is_seq(rq->q, blk_rq_pos(rq));
+}
+
 /*
  * Some commands like WRITE SAME have a payload or data transfer size which
  * is different from the size of the request.  Any driver that supports such
@@ -1582,6 +1620,16 @@ static inline unsigned int bdev_zone_sectors(struct block_device *bdev)
 	return 0;
 }
 
+static inline unsigned int bdev_nr_zones(struct block_device *bdev)
+{
+	struct request_queue *q = bdev_get_queue(bdev);
+
+	if (q)
+		return blk_queue_nr_zones(q);
+
+	return 0;
+}
+
 static inline int queue_dma_alignment(struct request_queue *q)
 {
 	return q ? q->dma_alignment : 511;
-- 
2.13.5

[PATCH V4 07/16] scsi: sd_zbc: Initialize device request queue zoned data

[Damien Le Moal]

Initialize the seq_zones bitmap and nr_zones field of the disk request
queue on disk revalidate. As the seq_zones bitmap allocation is
identical to the allocation of the zone write lock bitmap, introduce
the helper sd_zbc_alloc_zone_bitmap(). Using this helper, wait for the
disk capacity and number of zones to stabilize on the second
revalidation pass to allocate and initialize the bitmaps.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/sd_zbc.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 124 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 27793b9f54c0..50340c9d265b 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -586,8 +586,110 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
 	return 0;
 }
 
+/**
+ * sd_zbc_alloc_zone_bitmap - Allocate a zone bitmap (one bit per zone).
+ * @sdkp: The disk of the bitmap
+ */
+static inline unsigned long *sd_zbc_alloc_zone_bitmap(struct scsi_disk *sdkp)
+{
+	struct request_queue *q = sdkp->disk->queue;
+
+	return kzalloc_node(BITS_TO_LONGS(sdkp->nr_zones)
+			    * sizeof(unsigned long),
+			    GFP_KERNEL, q->node);
+}
+
+/**
+ * sd_zbc_setup_seq_zones - Initialize the disk request queue zone type bitmap.
+ * @sdkp: The disk of the bitmap
+ *
+ * Allocate a zone bitmap and initialize it by identifying sequential zones.
+ */
+static int sd_zbc_setup_seq_zones(struct scsi_disk *sdkp)
+{
+	struct request_queue *q = sdkp->disk->queue;
+	unsigned long *seq_zones;
+	sector_t block = 0;
+	unsigned char *buf;
+	unsigned char *rec;
+	unsigned int buf_len;
+	unsigned int list_length;
+	unsigned int n = 0;
+	u8 type, cond;
+	int ret = -ENOMEM;
+
+	kfree(q->seq_zones);
+	q->seq_zones = NULL;
+
+	seq_zones = sd_zbc_alloc_zone_bitmap(sdkp);
+	if (!seq_zones)
+		return -ENOMEM;
+
+	buf = kmalloc(SD_ZBC_BUF_SIZE, GFP_KERNEL);
+	if (!buf)
+		goto out;
+
+	while (block < sdkp->capacity) {
+
+		ret = sd_zbc_report_zones(sdkp, buf, SD_ZBC_BUF_SIZE, block);
+		if (ret)
+			goto out;
+
+		/*
+		 * Parse reported zone descriptors to find sequiential zones.
+		 * Since read-only and offline zones cannot be written, do not
+		 * mark them as sequential in the bitmap.
+		 */
+		list_length = get_unaligned_be32(&buf[0]) + 64;
+		rec = buf + 64;
+		buf_len = min(list_length, SD_ZBC_BUF_SIZE);
+		while (rec < buf + buf_len) {
+			type = rec[0] & 0x0f;
+			cond = (rec[1] >> 4) & 0xf;
+			if (type != ZBC_ZONE_TYPE_CONV &&
+			    cond != ZBC_ZONE_COND_READONLY &&
+			    cond != ZBC_ZONE_COND_OFFLINE)
+				set_bit(n, seq_zones);
+			block = get_unaligned_be64(&rec[8]) +
+				get_unaligned_be64(&rec[16]);
+			rec += 64;
+			n++;
+		}
+
+	}
+
+	if (n != sdkp->nr_zones) {
+		/* Something was wrong */
+		ret = -EIO;
+	}
+
+out:
+	kfree(buf);
+	if (ret) {
+		kfree(seq_zones);
+		return ret;
+	}
+
+	q->seq_zones = seq_zones;
+
+	return 0;
+}
+
+static void sd_zbc_cleanup(struct scsi_disk *sdkp)
+{
+	struct request_queue *q = sdkp->disk->queue;
+
+	kfree(q->seq_zones);
+	q->seq_zones = NULL;
+
+	kfree(sdkp->zones_wlock);
+	sdkp->zones_wlock = NULL;
+}
+
 static int sd_zbc_setup(struct scsi_disk *sdkp)
 {
+	struct request_queue *q = sdkp->disk->queue;
+	int ret;
 
 	/* READ16/WRITE16 is mandatory for ZBC disks */
 	sdkp->device->use_16_for_rw = 1;
@@ -599,14 +701,30 @@ static int sd_zbc_setup(struct scsi_disk *sdkp)
 	sdkp->nr_zones =
 		round_up(sdkp->capacity, sdkp->zone_blocks) >> sdkp->zone_shift;
 
-	if (!sdkp->zones_wlock) {
-		sdkp->zones_wlock = kcalloc(BITS_TO_LONGS(sdkp->nr_zones),
-					    sizeof(unsigned long),
-					    GFP_KERNEL);
+	/*
+	 * Wait for the disk capacity to stabilize before
+	 * initializing zone related information.
+	 */
+	if (sdkp->first_scan)
+		return 0;
+
+	if (!sdkp->zones_wlock || q->nr_zones != sdkp->nr_zones) {
+		kfree(sdkp->zones_wlock);
+		sdkp->zones_wlock = sd_zbc_alloc_zone_bitmap(sdkp);
 		if (!sdkp->zones_wlock)
 			return -ENOMEM;
 	}
 
+	if (!q->seq_zones || q->nr_zones != sdkp->nr_zones) {
+		ret = sd_zbc_setup_seq_zones(sdkp);
+		if (ret) {
+			sd_zbc_cleanup(sdkp);
+			return ret;
+		}
+	}
+
+	q->nr_zones = sdkp->nr_zones;
+
 	return 0;
 }
 
@@ -661,14 +779,14 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
 
 err:
 	sdkp->capacity = 0;
+	sd_zbc_cleanup(sdkp);
 
 	return ret;
 }
 
 void sd_zbc_remove(struct scsi_disk *sdkp)
 {
-	kfree(sdkp->zones_wlock);
-	sdkp->zones_wlock = NULL;
+	sd_zbc_cleanup(sdkp);
 }
 
 void sd_zbc_print_zones(struct scsi_disk *sdkp)
-- 
2.13.5

[PATCH V4 08/16] scsi: sd_zbc: Limit zone write locking to sequential zones

[Damien Le Moal]

Zoned block devices have no write constraints for conventional zones.
So write locking of conventional zones is not necessary and can even
hurt performance by unnecessarily operating the disk under low queue
depth. To avoid this, use the disk request queue seq_zones bitmap to
allow any write to be issued to conventional zones, locking only
sequential zones.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/sd_zbc.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 50340c9d265b..9bc0bde596e3 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -298,10 +298,11 @@ int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd)
 int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 {
 	struct request *rq = cmd->request;
+	struct request_queue *q = rq->q;
 	struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
 	sector_t sector = blk_rq_pos(rq);
 	sector_t zone_sectors = sd_zbc_zone_sectors(sdkp);
-	unsigned int zno = sd_zbc_zone_no(sdkp, sector);
+	unsigned int zno;
 
 	/*
 	 * Note: Checks of the alignment of the write command on
@@ -309,18 +310,21 @@ int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 	 */
 
 	/* Do not allow zone boundaries crossing on host-managed drives */
-	if (blk_queue_zoned_model(sdkp->disk->queue) == BLK_ZONED_HM &&
+	if (blk_queue_zoned_model(q) == BLK_ZONED_HM &&
 	    (sector & (zone_sectors - 1)) + blk_rq_sectors(rq) > zone_sectors)
 		return BLKPREP_KILL;
 
 	/*
-	 * Do not issue more than one write at a time per
-	 * zone. This solves write ordering problems due to
-	 * the unlocking of the request queue in the dispatch
-	 * path in the non scsi-mq case.
+	 * There is no write constraints on conventional zones. So any write
+	 * command can be sent. But do not issue more than one write command
+	 * at a time per sequential zone. This avoids write ordering problems
+	 * due to the unlocking of the request queue in the dispatch path of
+	 * legacy scsi path, as well as at the HBA level (e.g. AHCI).
 	 */
-	if (sdkp->zones_wlock &&
-	    test_and_set_bit(zno, sdkp->zones_wlock))
+	zno = sd_zbc_zone_no(sdkp, sector);
+	if (q->seq_zones && test_bit(zno, q->seq_zones))
+		return BLKPREP_OK;
+	if (sdkp->zones_wlock && test_and_set_bit(zno, sdkp->zones_wlock))
 		return BLKPREP_DEFER;
 
 	WARN_ON_ONCE(cmd->flags & SCMD_ZONE_WRITE_LOCK);
@@ -341,8 +345,9 @@ void sd_zbc_write_unlock_zone(struct scsi_cmnd *cmd)
 	struct request *rq = cmd->request;
 	struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
 
-	if (sdkp->zones_wlock && cmd->flags & SCMD_ZONE_WRITE_LOCK) {
+	if (cmd->flags & SCMD_ZONE_WRITE_LOCK) {
 		unsigned int zno = sd_zbc_zone_no(sdkp, blk_rq_pos(rq));
+
 		WARN_ON_ONCE(!test_bit(zno, sdkp->zones_wlock));
 		cmd->flags &= ~SCMD_ZONE_WRITE_LOCK;
 		clear_bit_unlock(zno, sdkp->zones_wlock);
-- 
2.13.5

[PATCH V4 09/16] scsi: sd_zbc: Disable zone write locking with scsi-mq

[Damien Le Moal]

In the case of a ZBC disk used with scsi-mq, zone write locking does
not prevent write reordering in sequential zones. Unlike the legacy
case, zone locking is done after the command request is removed from
the scheduler dispatch queue. That is, at the time of zone locking,
the write command may already be out of order, making locking
ineffective. Write order guarantees can only be provided by an
adapted I/O scheduler.

Disable zone write locking in sd_zbc_write_lock_zone() if the disk is
used with scsi-mq. As the disk zones_wlock bitmap is not necessry,
do not allocate it.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Bart Van Assche <Bart.VanAssche@wdc.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/sd_zbc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 9bc0bde596e3..34576fb31f3a 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -314,6 +314,10 @@ int sd_zbc_write_lock_zone(struct scsi_cmnd *cmd)
 	    (sector & (zone_sectors - 1)) + blk_rq_sectors(rq) > zone_sectors)
 		return BLKPREP_KILL;
 
+	/* No write locking with scsi-mq */
+	if (q->mq_ops)
+		return BLKPREP_OK;
+
 	/*
 	 * There is no write constraints on conventional zones. So any write
 	 * command can be sent. But do not issue more than one write command
@@ -713,7 +717,8 @@ static int sd_zbc_setup(struct scsi_disk *sdkp)
 	if (sdkp->first_scan)
 		return 0;
 
-	if (!sdkp->zones_wlock || q->nr_zones != sdkp->nr_zones) {
+	if (!q->mq_ops &&
+	    (!sdkp->zones_wlock || q->nr_zones != sdkp->nr_zones)) {
 		kfree(sdkp->zones_wlock);
 		sdkp->zones_wlock = sd_zbc_alloc_zone_bitmap(sdkp);
 		if (!sdkp->zones_wlock)
-- 
2.13.5

[PATCH V4 10/16] block: mq-deadline: Add zoned block device data

[Damien Le Moal]

Introduce new fields to mq-deadline private data to support zoned block
devices. The fields added provide a back pointer to the device request
queue to give access to the device zone model and zone information.
Also added are a zone bitmap used to implement zone write locking and
a spinlock to atomically handle zone locking with other processing.

Modify mq-dealine init_queue and exit_queue elevator methods to handle
initialization and cleanup of these fields.

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

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index a1cad4331edd..53b1d16179ad 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -60,6 +60,10 @@ struct deadline_data {
 
 	spinlock_t lock;
 	struct list_head dispatch;
+
+	struct request_queue *q;
+	spinlock_t zone_lock;
+	unsigned long *zones_wlock;
 };
 
 static inline struct rb_root *
@@ -300,6 +304,24 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	return rq;
 }
 
+static int deadline_enable_zones_wlock(struct deadline_data *dd,
+				       gfp_t gfp_mask)
+{
+	dd->zones_wlock = kzalloc_node(BITS_TO_LONGS(blk_queue_nr_zones(dd->q))
+				       * sizeof(unsigned long),
+				       gfp_mask, dd->q->node);
+	if (!dd->zones_wlock)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void deadline_disable_zones_wlock(struct deadline_data *dd)
+{
+	kfree(dd->zones_wlock);
+	dd->zones_wlock = NULL;
+}
+
 static void dd_exit_queue(struct elevator_queue *e)
 {
 	struct deadline_data *dd = e->elevator_data;
@@ -307,16 +329,40 @@ static void dd_exit_queue(struct elevator_queue *e)
 	BUG_ON(!list_empty(&dd->fifo_list[READ]));
 	BUG_ON(!list_empty(&dd->fifo_list[WRITE]));
 
+	deadline_disable_zones_wlock(dd);
 	kfree(dd);
 }
 
 /*
+ * initialize zoned block device related elevator private data.
+ */
+static int deadline_zoned_init_queue(struct request_queue *q,
+				     struct deadline_data *dd)
+{
+	if (!blk_queue_is_zoned(q) ||
+	    !blk_queue_nr_zones(q)) {
+		/*
+		 * Regular drive, or non-conforming zoned block device.
+		 * Do not use zone write locking.
+		 */
+		return 0;
+	}
+
+	/*
+	 * Enable zone write locking by default for any zoned
+	 * block device model.
+	 */
+	return deadline_enable_zones_wlock(dd, GFP_KERNEL);
+}
+
+/*
  * initialize elevator private data (deadline_data).
  */
 static int dd_init_queue(struct request_queue *q, struct elevator_type *e)
 {
 	struct deadline_data *dd;
 	struct elevator_queue *eq;
+	int ret;
 
 	eq = elevator_alloc(q, e);
 	if (!eq)
@@ -341,6 +387,15 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e)
 	spin_lock_init(&dd->lock);
 	INIT_LIST_HEAD(&dd->dispatch);
 
+	dd->q = q;
+	spin_lock_init(&dd->zone_lock);
+	ret = deadline_zoned_init_queue(q, dd);
+	if (ret) {
+		kfree(dd);
+		kobject_put(&eq->kobj);
+		return ret;
+	}
+
 	q->elevator = eq;
 	return 0;
 }
-- 
2.13.5

[PATCH V4 11/16] block: mq-deadline: Introduce zones_wlock attribute

[Damien Le Moal]

Zone write locking is mandatory for host managed zoned block devices so
that sequential write order can be maintained. This is however optional
for host aware devices as the device firmware can handle random writes.

The default initialization always enable zones write locking for any
zoned block device. Introduce the zones_wlock sysfs attribute to allow
users to change this default setting for host aware disks. Changes to
zones write locking state are not allowed for host managed disks (zones
write locking is always enabled) and for regular disks (zones write
locking is always disabled).

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/mq-deadline.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 53b1d16179ad..fb88fee0fb54 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -339,8 +339,7 @@ static void dd_exit_queue(struct elevator_queue *e)
 static int deadline_zoned_init_queue(struct request_queue *q,
 				     struct deadline_data *dd)
 {
-	if (!blk_queue_is_zoned(q) ||
-	    !blk_queue_nr_zones(q)) {
+	if (!blk_queue_is_zoned(q) || !blk_queue_nr_zones(q)) {
 		/*
 		 * Regular drive, or non-conforming zoned block device.
 		 * Do not use zone write locking.
@@ -559,6 +558,45 @@ STORE_FUNCTION(deadline_front_merges_store, &dd->front_merges, 0, 1, 0);
 STORE_FUNCTION(deadline_fifo_batch_store, &dd->fifo_batch, 0, INT_MAX, 0);
 #undef STORE_FUNCTION
 
+static ssize_t deadline_zones_wlock_show(struct elevator_queue *e, char *page)
+{
+	struct deadline_data *dd = e->elevator_data;
+
+	return deadline_var_show(dd->zones_wlock ? 1 : 0, page);
+}
+
+static ssize_t deadline_zones_wlock_store(struct elevator_queue *e,
+					  const char *page, size_t count)
+{
+	struct deadline_data *dd = e->elevator_data;
+	int ret, enable_zones_wlock;
+
+	deadline_var_store(&enable_zones_wlock, page);
+	if (enable_zones_wlock != 0 && enable_zones_wlock != 1)
+		return -EINVAL;
+
+	if ((enable_zones_wlock && dd->zones_wlock) ||
+	    (!enable_zones_wlock && !dd->zones_wlock))
+		return count;
+
+	/* Changes are allowed only for host-aware disks */
+	if (blk_queue_zoned_model(dd->q) != BLK_ZONED_HA)
+		return -EPERM;
+
+	blk_mq_freeze_queue(dd->q);
+
+	if (enable_zones_wlock) {
+		ret = deadline_enable_zones_wlock(dd, GFP_NOIO);
+	} else {
+		deadline_disable_zones_wlock(dd);
+		ret = 0;
+	}
+
+	blk_mq_unfreeze_queue(dd->q);
+
+	return ret ? ret : count;
+}
+
 #define DD_ATTR(name) \
 	__ATTR(name, S_IRUGO|S_IWUSR, deadline_##name##_show, \
 				      deadline_##name##_store)
@@ -569,6 +607,7 @@ static struct elv_fs_entry deadline_attrs[] = {
 	DD_ATTR(writes_starved),
 	DD_ATTR(front_merges),
 	DD_ATTR(fifo_batch),
+	DD_ATTR(zones_wlock),
 	__ATTR_NULL
 };
 
-- 
2.13.5

[PATCH V4 12/16] blokc: mq-deadline: Introduce dispatch helpers

[Damien Le Moal]

Avoid directly referencing the next_rq and fifo_list arrays using the
helper functions deadline_next_request() and deadline_fifo_request() to
facilitate changes in the dispatch request selection in
__dd_dispatch_request().

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/mq-deadline.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index fb88fee0fb54..1236a464b547 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -196,13 +196,36 @@ static inline int deadline_check_fifo(struct deadline_data *dd, int ddir)
 }
 
 /*
+ * For the specified data direction, return the next request to
+ * dispatch using arrival ordered lists.
+ */
+static struct request *
+deadline_fifo_request(struct deadline_data *dd, int data_dir)
+{
+	if (list_empty(&dd->fifo_list[data_dir]))
+		return NULL;
+
+	return rq_entry_fifo(dd->fifo_list[data_dir].next);
+}
+
+/*
+ * For the specified data direction, return the next request to
+ * dispatch using sector position sorted lists.
+ */
+static struct request *
+deadline_next_request(struct deadline_data *dd, int data_dir)
+{
+	return dd->next_rq[data_dir];
+}
+
+/*
  * deadline_dispatch_requests selects the best request according to
  * read/write expire, fifo_batch, etc
  */
 static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 {
 	struct deadline_data *dd = hctx->queue->elevator->elevator_data;
-	struct request *rq;
+	struct request *rq, *next_rq;
 	bool reads, writes;
 	int data_dir;
 
@@ -218,10 +241,9 @@ static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	/*
 	 * batches are currently reads XOR writes
 	 */
-	if (dd->next_rq[WRITE])
-		rq = dd->next_rq[WRITE];
-	else
-		rq = dd->next_rq[READ];
+	rq = deadline_next_request(dd, WRITE);
+	if (!rq)
+		rq = deadline_next_request(dd, READ);
 
 	if (rq && dd->batching < dd->fifo_batch)
 		/* we have a next request are still entitled to batch */
@@ -264,19 +286,20 @@ static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	/*
 	 * we are not running a batch, find best request for selected data_dir
 	 */
-	if (deadline_check_fifo(dd, data_dir) || !dd->next_rq[data_dir]) {
+	next_rq = deadline_next_request(dd, data_dir);
+	if (deadline_check_fifo(dd, data_dir) || !next_rq) {
 		/*
 		 * A deadline has expired, the last request was in the other
 		 * direction, or we have run out of higher-sectored requests.
 		 * Start again from the request with the earliest expiry time.
 		 */
-		rq = rq_entry_fifo(dd->fifo_list[data_dir].next);
+		rq = deadline_fifo_request(dd, data_dir);
 	} else {
 		/*
 		 * The last req was the same dir and we have a next request in
 		 * sort order. No expired requests so continue on from here.
 		 */
-		rq = dd->next_rq[data_dir];
+		rq = next_rq;
 	}
 
 	dd->batching = 0;
-- 
2.13.5

[PATCH V4 13/16] block: mq-deadline: Introduce zone locking support

[Damien Le Moal]

For a write request to a zoned block device, lock the request target
zone upon request displatch. The zone is unlocked either when the
request completes or when the request is requeued (inserted).

To indicate that a request has locked its target zone, use the first
pointer of the request elevator private data to store the value
RQ_ZONE_WLOCKED. Testing for this value allows quick decision in
dd_insert_request() and dd_completed_request() regarding the need for
unlocking the target zone of a request.

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

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 1236a464b547..2de1a36c34f5 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -179,6 +179,93 @@ deadline_move_request(struct deadline_data *dd, struct request *rq)
 }
 
 /*
+ * Return true if a request is a write requests that needs zone
+ * write locking.
+ */
+static inline bool deadline_request_needs_zone_wlock(struct deadline_data *dd,
+						     struct request *rq)
+{
+
+	if (!dd->zones_wlock)
+		return false;
+
+	if (blk_rq_is_passthrough(rq))
+		return false;
+
+	switch (req_op(rq)) {
+	case REQ_OP_WRITE_ZEROES:
+	case REQ_OP_WRITE_SAME:
+	case REQ_OP_WRITE:
+		return blk_rq_zone_is_seq(rq);
+	default:
+		return false;
+	}
+}
+
+/*
+ * Abuse the elv.priv[0] pointer to indicate if a request has write
+ * locked its target zone. Only write request to a zoned block device
+ * can own a zone write lock.
+ */
+#define RQ_ZONE_WLOCKED		((void *)1UL)
+static inline void deadline_set_request_zone_wlock(struct request *rq)
+{
+	rq->elv.priv[0] = RQ_ZONE_WLOCKED;
+}
+
+#define RQ_ZONE_NO_WLOCK	((void *)0UL)
+static inline void deadline_clear_request_zone_wlock(struct request *rq)
+{
+	rq->elv.priv[0] = RQ_ZONE_NO_WLOCK;
+}
+
+static inline bool deadline_request_has_zone_wlock(struct request *rq)
+{
+	return rq->elv.priv[0] == RQ_ZONE_WLOCKED;
+}
+
+/*
+ * Write lock the target zone of a write request.
+ */
+static void deadline_wlock_zone(struct deadline_data *dd,
+				struct request *rq)
+{
+	unsigned int zno = blk_rq_zone_no(rq);
+
+	WARN_ON_ONCE(deadline_request_has_zone_wlock(rq));
+	WARN_ON_ONCE(test_and_set_bit(zno, dd->zones_wlock));
+	deadline_set_request_zone_wlock(rq);
+}
+
+/*
+ * Write unlock the target zone of a write request.
+ */
+static void deadline_wunlock_zone(struct deadline_data *dd,
+				  struct request *rq)
+{
+	unsigned int zno = blk_rq_zone_no(rq);
+	unsigned long flags;
+
+	spin_lock_irqsave(&dd->zone_lock, flags);
+
+	WARN_ON_ONCE(!test_and_clear_bit(zno, dd->zones_wlock));
+	deadline_clear_request_zone_wlock(rq);
+
+	spin_unlock_irqrestore(&dd->zone_lock, flags);
+}
+
+/*
+ * Test the write lock state of the target zone of a write request.
+ */
+static inline bool deadline_zone_is_wlocked(struct deadline_data *dd,
+					    struct request *rq)
+{
+	unsigned int zno = blk_rq_zone_no(rq);
+
+	return test_bit(zno, dd->zones_wlock);
+}
+
+/*
  * deadline_check_fifo returns 0 if there are no expired requests on the fifo,
  * 1 otherwise. Requires !list_empty(&dd->fifo_list[data_dir])
  */
@@ -311,6 +398,11 @@ static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	dd->batching++;
 	deadline_move_request(dd, rq);
 done:
+	/*
+	 * If the request needs its target zone locked, do it.
+	 */
+	if (deadline_request_needs_zone_wlock(dd, rq))
+		deadline_wlock_zone(dd, rq);
 	rq->rq_flags |= RQF_STARTED;
 	return rq;
 }
@@ -472,6 +564,13 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 	struct deadline_data *dd = q->elevator->elevator_data;
 	const int data_dir = rq_data_dir(rq);
 
+	/*
+	 * This may be a requeue of a request that has locked its
+	 * target zone. If this is the case, release the request zone lock.
+	 */
+	if (deadline_request_has_zone_wlock(rq))
+		deadline_wunlock_zone(dd, rq);
+
 	if (blk_mq_sched_try_insert_merge(q, rq))
 		return;
 
@@ -516,6 +615,20 @@ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx,
 	spin_unlock(&dd->lock);
 }
 
+/*
+ * For zoned block devices, write unlock the target zone of
+ * completed write requests.
+ */
+static void dd_completed_request(struct request *rq)
+{
+
+	if (deadline_request_has_zone_wlock(rq)) {
+		struct deadline_data *dd = rq->q->elevator->elevator_data;
+
+		deadline_wunlock_zone(dd, rq);
+	}
+}
+
 static bool dd_has_work(struct blk_mq_hw_ctx *hctx)
 {
 	struct deadline_data *dd = hctx->queue->elevator->elevator_data;
@@ -757,6 +870,7 @@ static struct elevator_type mq_deadline = {
 	.ops.mq = {
 		.insert_requests	= dd_insert_requests,
 		.dispatch_request	= dd_dispatch_request,
+		.completed_request	= dd_completed_request,
 		.next_request		= elv_rb_latter_request,
 		.former_request		= elv_rb_former_request,
 		.bio_merge		= dd_bio_merge,
-- 
2.13.5

[PATCH V4 14/16] block: mq-deadline: Limit write dispatch for zoned block devices

[Damien Le Moal]

When dispatching writes to a zoned block device, only allow the request
to be dispatched if its target zone is not locked. If it is, leave the
request in the scheduler queues and look for another suitable write
request. If no write can be dispatched, allow reads to be dispatched
even if the write batch is not done.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/mq-deadline.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 2de1a36c34f5..8725b4f950c1 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -283,16 +283,44 @@ static inline int deadline_check_fifo(struct deadline_data *dd, int ddir)
 }
 
 /*
+ * Test if a request can be dispatched.
+ */
+static inline bool deadline_can_dispatch_request(struct deadline_data *dd,
+						 struct request *rq)
+{
+	if (!deadline_request_needs_zone_wlock(dd, rq))
+		return true;
+	return !deadline_zone_is_wlocked(dd, rq);
+}
+
+/*
  * For the specified data direction, return the next request to
  * dispatch using arrival ordered lists.
  */
 static struct request *
 deadline_fifo_request(struct deadline_data *dd, int data_dir)
 {
+	struct request *rq;
+	unsigned long flags;
+
 	if (list_empty(&dd->fifo_list[data_dir]))
 		return NULL;
 
-	return rq_entry_fifo(dd->fifo_list[data_dir].next);
+	if (!dd->zones_wlock || data_dir == READ)
+		return rq_entry_fifo(dd->fifo_list[data_dir].next);
+
+	spin_lock_irqsave(&dd->zone_lock, flags);
+
+	list_for_each_entry(rq, &dd->fifo_list[WRITE], queuelist) {
+		if (deadline_can_dispatch_request(dd, rq))
+			goto out;
+	}
+	rq = NULL;
+
+out:
+	spin_unlock_irqrestore(&dd->zone_lock, flags);
+
+	return rq;
 }
 
 /*
@@ -302,7 +330,21 @@ deadline_fifo_request(struct deadline_data *dd, int data_dir)
 static struct request *
 deadline_next_request(struct deadline_data *dd, int data_dir)
 {
-	return dd->next_rq[data_dir];
+	struct request *rq = dd->next_rq[data_dir];
+	unsigned long flags;
+
+	if (!dd->zones_wlock || data_dir == READ)
+		return rq;
+
+	spin_lock_irqsave(&dd->zone_lock, flags);
+	while (rq) {
+		if (deadline_can_dispatch_request(dd, rq))
+			break;
+		rq = deadline_latter_request(rq);
+	}
+	spin_unlock_irqrestore(&dd->zone_lock, flags);
+
+	return rq;
 }
 
 /*
@@ -344,7 +386,8 @@ static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	if (reads) {
 		BUG_ON(RB_EMPTY_ROOT(&dd->sort_list[READ]));
 
-		if (writes && (dd->starved++ >= dd->writes_starved))
+		if (deadline_fifo_request(dd, WRITE) &&
+		    (dd->starved++ >= dd->writes_starved))
 			goto dispatch_writes;
 
 		data_dir = READ;
@@ -389,6 +432,13 @@ static struct request *__dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 		rq = next_rq;
 	}
 
+	/*
+	 * If we only have writes queued and none of them can be dispatched,
+	 * rq will be NULL.
+	 */
+	if (!rq)
+		return NULL;
+
 	dd->batching = 0;
 
 dispatch_request:
@@ -566,7 +616,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 
 	/*
 	 * This may be a requeue of a request that has locked its
-	 * target zone. If this is the case, release the request zone lock.
+	 * target zone. If this is the case, release the zone lock.
 	 */
 	if (deadline_request_has_zone_wlock(rq))
 		deadline_wunlock_zone(dd, rq);
@@ -576,6 +626,9 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 
 	blk_mq_sched_request_inserted(rq);
 
+	if (at_head && deadline_request_needs_zone_wlock(dd, rq))
+		pr_info("######## Write at head !\n");
+
 	if (at_head || blk_rq_is_passthrough(rq)) {
 		if (at_head)
 			list_add(&rq->queuelist, &dd->dispatch);
-- 
2.13.5

[PATCH V4 15/16] block: do not set mq defaulte scheduler

[Damien Le Moal]

For blk-mq disks with a single hardware queue, setting by default the
disk scheduler to mq-deadline early during the queue initialization
prevents properly setting zone write locking for zoned block device as
the fact that the disk is zoned is not yet known.

Fix this by simply not setting the default scheduler to mq-deadline for
single hardware queue disks. udev can be used to easily do the same
later in the system initialization sequence, when the device
characteristics are known.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/elevator.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 153926a90901..8b65a757f726 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -222,19 +222,14 @@ int elevator_init(struct request_queue *q, char *name)
 
 	if (!e) {
 		/*
-		 * For blk-mq devices, we default to using mq-deadline,
-		 * if available, for single queue devices. If deadline
-		 * isn't available OR we have multiple queues, default
-		 * to "none".
+		 * For blk-mq devices, default to "none". udev can later set
+		 * an appropriate default scheduler based on the disk
+		 * characteristics which we do not yet have here.
 		 */
-		if (q->mq_ops) {
-			if (q->nr_hw_queues == 1)
-				e = elevator_get("mq-deadline", false);
-			if (!e)
-				return 0;
-		} else
-			e = elevator_get(CONFIG_DEFAULT_IOSCHED, false);
+		if (q->mq_ops)
+			return 0;
 
+		e = elevator_get(CONFIG_DEFAULT_IOSCHED, false);
 		if (!e) {
 			printk(KERN_ERR
 				"Default I/O scheduler not found. " \
-- 
2.13.5

[PATCH V4 16/16] block: mq-deadline: Update documentation

[Damien Le Moal]

Added description of the zones_wlock tunable attribute.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 Documentation/block/deadline-iosched.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/block/deadline-iosched.txt b/Documentation/block/deadline-iosched.txt
index 2d82c80322cb..b532304d2bf4 100644
--- a/Documentation/block/deadline-iosched.txt
+++ b/Documentation/block/deadline-iosched.txt
@@ -69,6 +69,23 @@ Front merges may still occur due to the cached last_merge hint, but since
 that comes at basically 0 cost we leave that on. We simply disable the
 rbtree front sector lookup when the io scheduler merge function is called.
 
+zones_wlock	(bool)
+-----------
+
+(mq-deadlines only)
+The blk-mq version of the deadline I/O scheduler, mq-deadline, introduced
+support for zoned block devices. This added support prevents dispatching of more
+than one write command per zone of the device to avoid write sequence reordering
+due to the possible concurrent execution of many blk-mq operations, in
+particular request dispatching. This is achieved using a per zone lock
+implemented as a bitmap. When a write request is dispatched, the target zone of
+the  request is locked, preventing further dispatch of write requests to it
+(there are no limitations on read commands). The zones_wlock tunable allows
+disabling zone write locking for host aware zoned block devices as these drive
+can handle random writes to zones in firmware. This tunable default setting
+cannot be changed for host managed disks (always enabled) and regular disks
+(always disabled).
+
 
 Nov 11 2002, Jens Axboe <jens.axboe@oracle.com>
 
-- 
2.13.5

Re: [PATCH V4 02/16] scsi: sd_zbc: Fix comments and indentation

[Christoph Hellwig]

Looks fine,

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

Re: [PATCH V4 06/16] block: Add zoned block device information to request queue

[Christoph Hellwig]

> +	 * Zoned block device information for mq I/O schedulers.
> +	 * Set by low level device driver (stacking driver may not set this).
> +	 */
> +	unsigned int	nr_zones;
> +	unsigned long	*seq_zones;

Might be worth explaining the seq_zones is a bit bitmap to check
if a given zone number is sequential.

And maybe give it a name that hints at that, like seq_zone_bitmap?

Otherwise looks fine:

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

Re: [PATCH V4 00/16] scsi-mq support for ZBC disks

[Christoph Hellwig]

>  Documentation/block/zoned-iosched.txt |  48 ++
>  block/Kconfig.iosched                 |  12 +
>  block/Makefile                        |   1 +
>  block/blk-mq-debugfs.h                |  14 +-
>  block/blk-mq-sched.h                  |  11 +-
>  block/zoned-iosched.c                 | 925 ++++++++++++++++++++++++++++++++++
>  drivers/scsi/scsi_lib.c               |   5 +-
>  drivers/scsi/sd_zbc.c                 | 267 +++++++---
>  include/linux/blk-mq-debugfs.h        |  23 +
>  include/linux/blk-mq-sched.h          |  14 +
>  include/linux/blkdev.h                |  24 +
>  include/scsi/scsi_proto.h             |  45 +-
>  12 files changed, 1283 insertions(+), 106 deletions(-)
>  create mode 100644 Documentation/block/zoned-iosched.txt
>  create mode 100644 block/zoned-iosched.c
>  create mode 100644 include/linux/blk-mq-debugfs.h
>  create mode 100644 include/linux/blk-mq-sched.h

This still seems to document the previous version.  Did it get
any smaller by merging the zoned schedule into mq-deadline?

Re: [PATCH V4 07/16] scsi: sd_zbc: Initialize device request queue zoned data

[Christoph Hellwig]

> +static inline unsigned long *sd_zbc_alloc_zone_bitmap(struct scsi_disk *sdkp)
> +{
> +	struct request_queue *q = sdkp->disk->queue;
> +
> +	return kzalloc_node(BITS_TO_LONGS(sdkp->nr_zones)
> +			    * sizeof(unsigned long),
> +			    GFP_KERNEL, q->node);

This really screams for kcalloc_node and friends that I think Johannes
volunteered to add.

> + * sd_zbc_setup_seq_zones - Initialize the disk request queue zone type bitmap.
> + * @sdkp: The disk of the bitmap
> + *
> + * Allocate a zone bitmap and initialize it by identifying sequential zones.
> + */
> +static int sd_zbc_setup_seq_zones(struct scsi_disk *sdkp)
> +{
> +	struct request_queue *q = sdkp->disk->queue;
> +	unsigned long *seq_zones;
> +	sector_t block = 0;
> +	unsigned char *buf;
> +	unsigned char *rec;
> +	unsigned int buf_len;
> +	unsigned int list_length;
> +	unsigned int n = 0;
> +	u8 type, cond;
> +	int ret = -ENOMEM;
> +
> +	kfree(q->seq_zones);
> +	q->seq_zones = NULL;

We also free the previous version, which isn't documented above.
Which in general begs the question:  What scheme protects access
to q->seq_zones?

And the previous patch should probably grow a comment to document
that q->seq_zones is entirely managed by the driver.

> +		/*
> +		 * Parse reported zone descriptors to find sequiential zones.
> +		 * Since read-only and offline zones cannot be written, do not
> +		 * mark them as sequential in the bitmap.
> +		 */
> +		list_length = get_unaligned_be32(&buf[0]) + 64;
> +		rec = buf + 64;
> +		buf_len = min(list_length, SD_ZBC_BUF_SIZE);
> +		while (rec < buf + buf_len) {
> +			type = rec[0] & 0x0f;
> +			cond = (rec[1] >> 4) & 0xf;
> +			if (type != ZBC_ZONE_TYPE_CONV &&
> +			    cond != ZBC_ZONE_COND_READONLY &&
> +			    cond != ZBC_ZONE_COND_OFFLINE)
> +				set_bit(n, seq_zones);
> +			block = get_unaligned_be64(&rec[8]) +
> +				get_unaligned_be64(&rec[16]);
> +			rec += 64;
> +			n++;
> +		}

Split this out into a helper?

Re: [PATCH V4 10/16] block: mq-deadline: Add zoned block device data

[Christoph Hellwig]

> +
> +	struct request_queue *q;

Do you really need the queue backpointer?  At least as far as this
patch is concerned we could just pass the queue on to
deadline_enable_zones_wlock and be fine.  And in general we should
always passing the q, as we can trivial go from queue to deadline_data
using queue->elevator->elevator_data.

> +static int deadline_zoned_init_queue(struct request_queue *q,
> +				     struct deadline_data *dd)
> +{
> +	if (!blk_queue_is_zoned(q) ||
> +	    !blk_queue_nr_zones(q)) {

Shouldn't !blk_queue_nr_zones(q) be enough?  If not both conditionals
could easily fit into the same line, and I'd be tempted to move them
to the caller and call deadline_enable_zones_wlock straight from there.

> @@ -341,6 +387,15 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e)
>  	spin_lock_init(&dd->lock);
>  	INIT_LIST_HEAD(&dd->dispatch);
>  
> +	dd->q = q;
> +	spin_lock_init(&dd->zone_lock);
> +	ret = deadline_zoned_init_queue(q, dd);
> +	if (ret) {
> +		kfree(dd);
> +		kobject_put(&eq->kobj);
> +		return ret;
> +	}
> +
>  	q->elevator = eq;
>  	return 0;

This should probably grow goto based unwinding, e.g.

	int ret = -ENOMEM;

	...

	dd = kzalloc_node(sizeof(*dd), GFP_KERNEL, q->node);
	if (!dd)
		goto out_put_object;

	...

	if (blk_queue_nr_zones(q))) {
		ret = deadline_enable_zones_wlock(...);
		if (ret)
			goto out_free_dd;
	}
	
  	q->elevator = eq;
  	return 0;

out_free_dd:
	kfree(dd);
out_put_object
	kobject_put(&eq->kobj);
	return ret;
	

Re: [PATCH V4 09/16] scsi: sd_zbc: Disable zone write locking with scsi-mq

[Christoph Hellwig]

Looks fine,

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

Re: [PATCH V4 08/16] scsi: sd_zbc: Limit zone write locking to sequential zones

[Christoph Hellwig]

> +	if (q->seq_zones && test_bit(zno, q->seq_zones))
> +		return BLKPREP_OK;

Isn't the check above inverted?  Also shouldn't it use blk_rq_zone_is_seq?
E.g.

	if (!blk_rq_zone_is_seq(cmd->request))
		return BLKPREP_OK;
?

Re: [PATCH V4 11/16] block: mq-deadline: Introduce zones_wlock attribute

[Christoph Hellwig]

On Sun, Sep 24, 2017 at 04:02:42PM +0900, Damien Le Moal wrote:
> Zone write locking is mandatory for host managed zoned block devices so
> that sequential write order can be maintained. This is however optional
> for host aware devices as the device firmware can handle random writes.
> 
> The default initialization always enable zones write locking for any
> zoned block device. Introduce the zones_wlock sysfs attribute to allow
> users to change this default setting for host aware disks. Changes to
> zones write locking state are not allowed for host managed disks (zones
> write locking is always enabled) and for regular disks (zones write
> locking is always disabled).

Is it really worth the effort?  Until someone who really ares about
HA drives and shows a benefit for a workload that matters I'd just
leave to code off for HA.

Re: [PATCH V4 12/16] blokc: mq-deadline: Introduce dispatch helpers

[Christoph Hellwig]

> +static struct request *
> +deadline_next_request(struct deadline_data *dd, int data_dir)
> +{

	if (WARN_ON_ONCE(data_dir != READ && data_dir != WRITE))
		return NULL;

> +	return dd->next_rq[data_dir];
> +}
> +

Else looks fine to me:

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

Re: [PATCH V4 06/16] block: Add zoned block device information to request queue

[Damien Le Moal]

On 9/24/17 16:59, Christoph Hellwig wrote:
>> +	 * Zoned block device information for mq I/O schedulers.
>> +	 * Set by low level device driver (stacking driver may not set this).
>> +	 */
>> +	unsigned int	nr_zones;
>> +	unsigned long	*seq_zones;
> 
> Might be worth explaining the seq_zones is a bit bitmap to check
> if a given zone number is sequential.
> 
> And maybe give it a name that hints at that, like seq_zone_bitmap?
> 
> Otherwise looks fine:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks. Will send a V5 with your suggested modifications.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 00/16] scsi-mq support for ZBC disks

[Damien Le Moal]

On 9/24/17 17:02, Christoph Hellwig wrote:
>>  Documentation/block/zoned-iosched.txt |  48 ++
>>  block/Kconfig.iosched                 |  12 +
>>  block/Makefile                        |   1 +
>>  block/blk-mq-debugfs.h                |  14 +-
>>  block/blk-mq-sched.h                  |  11 +-
>>  block/zoned-iosched.c                 | 925 ++++++++++++++++++++++++++++++++++
>>  drivers/scsi/scsi_lib.c               |   5 +-
>>  drivers/scsi/sd_zbc.c                 | 267 +++++++---
>>  include/linux/blk-mq-debugfs.h        |  23 +
>>  include/linux/blk-mq-sched.h          |  14 +
>>  include/linux/blkdev.h                |  24 +
>>  include/scsi/scsi_proto.h             |  45 +-
>>  12 files changed, 1283 insertions(+), 106 deletions(-)
>>  create mode 100644 Documentation/block/zoned-iosched.txt
>>  create mode 100644 block/zoned-iosched.c
>>  create mode 100644 include/linux/blk-mq-debugfs.h
>>  create mode 100644 include/linux/blk-mq-sched.h
> 
> This still seems to document the previous version.  Did it get
> any smaller by merging the zoned schedule into mq-deadline?

Ooops... Sorry. I messed up the cover letter. Here are the changes:

Damien Le Moal (16):

  scsi: sd_zbc: Move ZBC declarations to scsi_proto.h

  scsi: sd_zbc: Fix comments and indentation

  scsi: sd_zbc: Rearrange code

  scsi: sd_zbc: Use well defined macros

  scsi: sd_zbc: Fix sd_zbc_read_zoned_characteristics()

  block: Add zoned block device information to request queue

  scsi: sd_zbc: Initialize device request queue zoned data

  scsi: sd_zbc: Limit zone write locking to sequential zones

  scsi: sd_zbc: Disable zone write locking with scsi-mq

  block: mq-deadline: Add zoned block device data

  block: mq-deadline: Introduce zones_wlock attribute

  blokc: mq-deadline: Introduce dispatch helpers

  block: mq-deadline: Introduce zone locking support

  block: mq-deadline: Limit write dispatch for zoned block devices

  block: do not set mq defaulte scheduler

  block: mq-deadline: Update documentation



 Documentation/block/deadline-iosched.txt |  17 ++

 block/elevator.c                         |  17 +-

 block/mq-deadline.c                      | 302
+++++++++++++++++++++++++++-
 drivers/scsi/scsi_lib.c                  |   5 +-

 drivers/scsi/sd_zbc.c                    | 325
++++++++++++++++++++++++-------
 include/linux/blkdev.h                   |  48 +++++

 include/scsi/scsi_proto.h                |  45 +++--

 7 files changed, 656 insertions(+), 103 deletions(-)

I resent the cover letter.

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 07/16] scsi: sd_zbc: Initialize device request queue zoned data

[Damien Le Moal]

On 9/24/17 17:07, Christoph Hellwig wrote:
>> +static inline unsigned long *sd_zbc_alloc_zone_bitmap(struct scsi_disk *sdkp)
>> +{
>> +	struct request_queue *q = sdkp->disk->queue;
>> +
>> +	return kzalloc_node(BITS_TO_LONGS(sdkp->nr_zones)
>> +			    * sizeof(unsigned long),
>> +			    GFP_KERNEL, q->node);
> 
> This really screams for kcalloc_node and friends that I think Johannes
> volunteered to add.

OK. Should I wait for Johannes patches ? That can be easily changed
later though.

>> + * sd_zbc_setup_seq_zones - Initialize the disk request queue zone type bitmap.
>> + * @sdkp: The disk of the bitmap
>> + *
>> + * Allocate a zone bitmap and initialize it by identifying sequential zones.
>> + */
>> +static int sd_zbc_setup_seq_zones(struct scsi_disk *sdkp)
>> +{
>> +	struct request_queue *q = sdkp->disk->queue;
>> +	unsigned long *seq_zones;
>> +	sector_t block = 0;
>> +	unsigned char *buf;
>> +	unsigned char *rec;
>> +	unsigned int buf_len;
>> +	unsigned int list_length;
>> +	unsigned int n = 0;
>> +	u8 type, cond;
>> +	int ret = -ENOMEM;
>> +
>> +	kfree(q->seq_zones);
>> +	q->seq_zones = NULL;
> 
> We also free the previous version, which isn't documented above.
> Which in general begs the question:  What scheme protects access
> to q->seq_zones?

Yes, indeed, the comments do not mention that. I will add that.
As for the protection, there is none necessary I think. The reason is
that the previous version free+alloc can only happen if sd_revalidate is
called, at which point there are no write commands on-going, so no
references to seq_zones. Is this correct/not correct ?

I am not even sure if the reallocation/reinit is even necessary though.
Since sd_revalidate() will at worst result in the disk capacity going to
0 (if the disk is non responsive for instance), accesses beyond
seq_zones size will never happen. And the zone types never change, so it
may be better to drop this reallocation+reinit. Same for the zones_wlock
bitmap. What do you hink ?

> And the previous patch should probably grow a comment to document
> that q->seq_zones is entirely managed by the driver.

Will do.

>> +		/*
>> +		 * Parse reported zone descriptors to find sequiential zones.
>> +		 * Since read-only and offline zones cannot be written, do not
>> +		 * mark them as sequential in the bitmap.
>> +		 */
>> +		list_length = get_unaligned_be32(&buf[0]) + 64;
>> +		rec = buf + 64;
>> +		buf_len = min(list_length, SD_ZBC_BUF_SIZE);
>> +		while (rec < buf + buf_len) {
>> +			type = rec[0] & 0x0f;
>> +			cond = (rec[1] >> 4) & 0xf;
>> +			if (type != ZBC_ZONE_TYPE_CONV &&
>> +			    cond != ZBC_ZONE_COND_READONLY &&
>> +			    cond != ZBC_ZONE_COND_OFFLINE)
>> +				set_bit(n, seq_zones);
>> +			block = get_unaligned_be64(&rec[8]) +
>> +				get_unaligned_be64(&rec[16]);
>> +			rec += 64;
>> +			n++;
>> +		}
> 
> Split this out into a helper?

Yes, that would be a nice cleanup. Will do.

Thanks.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 10/16] block: mq-deadline: Add zoned block device data

[Damien Le Moal]

On 9/24/17 17:14, Christoph Hellwig wrote:
>> +
>> +	struct request_queue *q;
> 
> Do you really need the queue backpointer?  At least as far as this
> patch is concerned we could just pass the queue on to
> deadline_enable_zones_wlock and be fine.  And in general we should
> always passing the q, as we can trivial go from queue to deadline_data
> using queue->elevator->elevator_data.

This is for the sysfs zones_wlock store function which does not give the
queue. Instead of this backpointer, I can copy the queue node, number of
zones and zone model so that cdeadline_enable_zones_wlock() can be
called equally from init_queue context and from the sysfs zones_wlock
store context.

>> +static int deadline_zoned_init_queue(struct request_queue *q,
>> +				     struct deadline_data *dd)
>> +{
>> +	if (!blk_queue_is_zoned(q) ||
>> +	    !blk_queue_nr_zones(q)) {
> 
> Shouldn't !blk_queue_nr_zones(q) be enough?  If not both conditionals
> could easily fit into the same line, and I'd be tempted to move them
> to the caller and call deadline_enable_zones_wlock straight from there.

OK. Will update.

>> @@ -341,6 +387,15 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e)
>>  	spin_lock_init(&dd->lock);
>>  	INIT_LIST_HEAD(&dd->dispatch);
>>  
>> +	dd->q = q;
>> +	spin_lock_init(&dd->zone_lock);
>> +	ret = deadline_zoned_init_queue(q, dd);
>> +	if (ret) {
>> +		kfree(dd);
>> +		kobject_put(&eq->kobj);
>> +		return ret;
>> +	}
>> +
>>  	q->elevator = eq;
>>  	return 0;
> 
> This should probably grow goto based unwinding, e.g.

OK. Will update.

Thanks !

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 08/16] scsi: sd_zbc: Limit zone write locking to sequential zones

[Damien Le Moal]

On 9/24/17 17:18, Christoph Hellwig wrote:
>> +	if (q->seq_zones && test_bit(zno, q->seq_zones))
>> +		return BLKPREP_OK;
> 
> Isn't the check above inverted?  Also shouldn't it use blk_rq_zone_is_seq?
> E.g.
> 
> 	if (!blk_rq_zone_is_seq(cmd->request))
> 		return BLKPREP_OK;
> ?

Arrg ! Good catch. I tested only the scsi-mq case which does not use
this path anymore (same test used, but at the scheduler level). Silly of
me. I should have properly tested all cases.

Will fix this.

Thanks.

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 11/16] block: mq-deadline: Introduce zones_wlock attribute

[Damien Le Moal]

On 9/24/17 17:19, Christoph Hellwig wrote:
> On Sun, Sep 24, 2017 at 04:02:42PM +0900, Damien Le Moal wrote:
>> Zone write locking is mandatory for host managed zoned block devices so
>> that sequential write order can be maintained. This is however optional
>> for host aware devices as the device firmware can handle random writes.
>>
>> The default initialization always enable zones write locking for any
>> zoned block device. Introduce the zones_wlock sysfs attribute to allow
>> users to change this default setting for host aware disks. Changes to
>> zones write locking state are not allowed for host managed disks (zones
>> write locking is always enabled) and for regular disks (zones write
>> locking is always disabled).
> 
> Is it really worth the effort?  Until someone who really ares about
> HA drives and shows a benefit for a workload that matters I'd just
> leave to code off for HA.

OK. Will do. That can be changed back later again easily enough.

Thanks.

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH V4 02/16] scsi: sd_zbc: Fix comments and indentation

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH V4 02/16] scsi: sd_zbc: Fix comments and indentation

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH V4 03/16] scsi: sd_zbc: Rearrange code

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH V4 03/16] scsi: sd_zbc: Rearrange code

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH V4 07/16] scsi: sd_zbc: Initialize device request queue zoned data

[Johannes Thumshirn]

On Sun, Sep 24, 2017 at 05:07:01PM +0200, Christoph Hellwig wrote:
> > +static inline unsigned long *sd_zbc_alloc_zone_bitmap(struct scsi_disk *sdkp)
> > +{
> > +	struct request_queue *q = sdkp->disk->queue;
> > +
> > +	return kzalloc_node(BITS_TO_LONGS(sdkp->nr_zones)
> > +			    * sizeof(unsigned long),
> > +			    GFP_KERNEL, q->node);
> 
> This really screams for kcalloc_node and friends that I think Johannes
> volunteered to add.

Thanks for the reminder, I'll send out the series today.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH V4 07/16] scsi: sd_zbc: Initialize device request queue zoned data

[Johannes Thumshirn]

On Sun, Sep 24, 2017 at 05:07:01PM +0200, Christoph Hellwig wrote:
> > +static inline unsigned long *sd_zbc_alloc_zone_bitmap(struct scsi_disk *sdkp)
> > +{
> > +	struct request_queue *q = sdkp->disk->queue;
> > +
> > +	return kzalloc_node(BITS_TO_LONGS(sdkp->nr_zones)
> > +			    * sizeof(unsigned long),
> > +			    GFP_KERNEL, q->node);
> 
> This really screams for kcalloc_node and friends that I think Johannes
> volunteered to add.

Thanks for the reminder, I'll send out the series today.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850