[PATCH 00/24] Optimize the hot path in the UFS driver

[PATCH 00/24] Optimize the hot path in the UFS driver

[Bart Van Assche]

Hi Martin,

This patch series increases IOPS by 1% and reduces CPU per I/O by 10% on my
UFS 4.0 test setup. Please consider this patch series for the next merge
window.

Thanks,

Bart.

Bart Van Assche (23):
  scsi: core: Make scsi_cmd_to_rq() accept const arguments
  scsi: core: Make scsi_cmd_priv() accept const arguments
  scsi: core: Use scsi_cmd_priv() instead of open-coding it
  scsi: core: Introduce scsi_host_update_can_queue()
  scsi: ufs: core: Change the type of one ufshcd_add_cmd_upiu_trace()
    argument
  scsi: ufs: core: Only call ufshcd_add_command_trace() for SCSI
    commands
  scsi: ufs: core: Change the type of one ufshcd_add_command_trace()
    argument
  scsi: ufs: core: Change the type of one ufshcd_send_command() argument
  scsi: ufs: core: Only call ufshcd_should_inform_monitor() for SCSI
    commands
  scsi: ufs: core: Change the monitor function argument types
  scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()
  scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()
  scsi: ufs: core: Cache the DMA buffer sizes
  scsi: ufs: core: Add an argument to ufshcd_mcq_decide_queue_depth()
  scsi: ufs: core: Add an argument to ufshcd_alloc_mcq()
  scsi: ufs: core: Call ufshcd_mcq_init() once
  scsi: ufs: core: Allocate the SCSI host earlier
  scsi: ufs: core: Call ufshcd_init_lrb() later
  scsi: ufs: core: Use hba->reserved_slot
  scsi: ufs: core: Allocate the reserved slot as a reserved request
  scsi: ufs: core: Do not clear driver-private command data
  scsi: ufs: core: Optimize the hot path
  scsi: ufs: core: Remove the ufshcd_lrb task_tag member

Hannes Reinecke (1):
  scsi: core: Implement reserved command handling

 drivers/scsi/hosts.c             |   3 +
 drivers/scsi/scsi.c              |  26 ++
 drivers/scsi/scsi_lib.c          |   6 +-
 drivers/scsi/scsi_logging.c      |  10 +-
 drivers/ufs/core/ufs-mcq.c       |  31 +-
 drivers/ufs/core/ufshcd-crypto.h |  18 +-
 drivers/ufs/core/ufshcd-priv.h   |  27 +-
 drivers/ufs/core/ufshcd.c        | 660 +++++++++++++++++--------------
 include/scsi/scsi_cmnd.h         |  17 +-
 include/scsi/scsi_host.h         |  24 +-
 include/ufs/ufshcd.h             |  11 +-
 11 files changed, 487 insertions(+), 346 deletions(-)

[PATCH 01/24] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Bart Van Assche]

Instead of requiring the caller to cast away constness, make
scsi_cmd_to_rq() accept const arguments.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_logging.c | 10 +++++-----
 include/scsi/scsi_cmnd.h    |  9 +++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index b02af340c2d3..5aaff629b999 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -28,7 +28,7 @@ static void scsi_log_release_buffer(char *bufptr)
 
 static inline const char *scmd_name(const struct scsi_cmnd *scmd)
 {
-	struct request *rq = scsi_cmd_to_rq((struct scsi_cmnd *)scmd);
+	const struct request *rq = scsi_cmd_to_rq(scmd);
 
 	if (!rq->q || !rq->q->disk)
 		return NULL;
@@ -94,7 +94,7 @@ void scmd_printk(const char *level, const struct scsi_cmnd *scmd,
 	if (!logbuf)
 		return;
 	off = sdev_format_header(logbuf, logbuf_len, scmd_name(scmd),
-				 scsi_cmd_to_rq((struct scsi_cmnd *)scmd)->tag);
+				 scsi_cmd_to_rq(scmd)->tag);
 	if (off < logbuf_len) {
 		va_start(args, fmt);
 		off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
@@ -374,8 +374,8 @@ EXPORT_SYMBOL(__scsi_print_sense);
 void scsi_print_sense(const struct scsi_cmnd *cmd)
 {
 	scsi_log_print_sense(cmd->device, scmd_name(cmd),
-			     scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag,
-			     cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
+			     scsi_cmd_to_rq(cmd)->tag, cmd->sense_buffer,
+			     SCSI_SENSE_BUFFERSIZE);
 }
 EXPORT_SYMBOL(scsi_print_sense);
 
@@ -393,7 +393,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
 		return;
 
 	off = sdev_format_header(logbuf, logbuf_len, scmd_name(cmd),
-				 scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag);
+				 scsi_cmd_to_rq(cmd)->tag);
 
 	if (off >= logbuf_len)
 		goto out_printk;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 8ecfb94049db..154fbb39ca0c 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -144,10 +144,11 @@ struct scsi_cmnd {
 };
 
 /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */
-static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
-{
-	return blk_mq_rq_from_pdu(scmd);
-}
+#define scsi_cmd_to_rq(scmd)                                       \
+	_Generic(scmd,                                             \
+		const struct scsi_cmnd *: (const struct request *) \
+			blk_mq_rq_from_pdu((void *)scmd),          \
+		struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
 
 /*
  * Return the driver private allocation behind the command.

[PATCH 02/24] scsi: core: Make scsi_cmd_priv() accept const arguments

[Bart Van Assche]

Instead of requiring the caller to cast away constness, make
scsi_cmd_priv() accept const arguments.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/scsi/scsi_cmnd.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 154fbb39ca0c..09176b07e891 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -154,10 +154,10 @@ struct scsi_cmnd {
  * Return the driver private allocation behind the command.
  * Only works if cmd_size is set in the host template.
  */
-static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
-{
-	return cmd + 1;
-}
+#define scsi_cmd_priv(cmd)                                         \
+	_Generic(cmd,                                              \
+		const struct scsi_cmnd *: (const void *)(cmd + 1), \
+		struct scsi_cmnd *: (void *)(cmd + 1))
 
 void scsi_done(struct scsi_cmnd *cmd);
 void scsi_done_direct(struct scsi_cmnd *cmd);

[PATCH 03/24] scsi: core: Use scsi_cmd_priv() instead of open-coding it

[Bart Van Assche]

Improve code readability without modifying the behavior of the code.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index be0890e4e706..1da26f300287 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1674,7 +1674,7 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
 	 * a function to initialize that data.
 	 */
 	if (!shost->hostt->init_cmd_priv)
-		memset(cmd + 1, 0, shost->hostt->cmd_size);
+		memset(scsi_cmd_priv(cmd), 0, shost->hostt->cmd_size);
 
 	cmd->prot_op = SCSI_PROT_NORMAL;
 	if (blk_rq_bytes(req))

[PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

From: Hannes Reinecke <hare@suse.de>

Quite some drivers are using management commands internally. These commands
typically use the same tag pool as regular SCSI commands. Tags for these
management commands are set aside before allocating the block-mq tag bitmap
for regular SCSI commands. The block layer already supports this via the
reserved tag mechanism. Add a new field 'nr_reserved_cmds' to the SCSI host
template to instruct the block layer to set aside a tag space for these
management commands by using reserved tags. Exclude reserved commands from
.can_queue because .can_queue is visible in sysfs.

Signed-off-by: Hannes Reinecke <hare@suse.de>
[ bvanassche: modified patch description ]
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/hosts.c     |  3 +++
 drivers/scsi/scsi_lib.c  |  4 +++-
 include/scsi/scsi_host.h | 22 +++++++++++++++++++++-
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index e021f1106bea..c2c6c96781e3 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -495,6 +495,9 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 	if (sht->virt_boundary_mask)
 		shost->virt_boundary_mask = sht->virt_boundary_mask;
 
+	if (sht->nr_reserved_cmds)
+		shost->nr_reserved_cmds = sht->nr_reserved_cmds;
+
 	device_initialize(&shost->shost_gendev);
 	dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
 	shost->shost_gendev.bus = &scsi_bus_type;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1da26f300287..94dafa5ceaaa 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2083,7 +2083,9 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 		tag_set->ops = &scsi_mq_ops_no_commit;
 	tag_set->nr_hw_queues = shost->nr_hw_queues ? : 1;
 	tag_set->nr_maps = shost->nr_maps ? : 1;
-	tag_set->queue_depth = shost->can_queue;
+	tag_set->queue_depth =
+		shost->can_queue + shost->nr_reserved_cmds;
+	tag_set->reserved_tags = shost->nr_reserved_cmds;
 	tag_set->cmd_size = cmd_size;
 	tag_set->numa_node = dev_to_node(shost->dma_dev);
 	if (shost->hostt->tag_alloc_policy_rr)
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 26bc23419cfd..2c0f5ec1046e 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -375,10 +375,19 @@ struct scsi_host_template {
 	/*
 	 * This determines if we will use a non-interrupt driven
 	 * or an interrupt driven scheme.  It is set to the maximum number
-	 * of simultaneous commands a single hw queue in HBA will accept.
+	 * of simultaneous commands a single hw queue in HBA will accept
+	 * excluding internal commands.
 	 */
 	int can_queue;
 
+	/*
+	 * This determines how many commands the HBA will set aside
+	 * for internal commands. This number will be added to
+	 * @can_queue to calcumate the maximum number of simultaneous
+	 * commands sent to the host.
+	 */
+	int nr_reserved_cmds;
+
 	/*
 	 * In many instances, especially where disconnect / reconnect are
 	 * supported, our host also has an ID on the SCSI bus.  If this is
@@ -611,6 +620,11 @@ struct Scsi_Host {
 	unsigned short max_cmd_len;
 
 	int this_id;
+
+	/*
+	 * Number of commands this host can handle at the same time.
+	 * This excludes reserved commands as specified by nr_reserved_cmds.
+	 */
 	int can_queue;
 	short cmd_per_lun;
 	short unsigned int sg_tablesize;
@@ -631,6 +645,12 @@ struct Scsi_Host {
 	 */
 	unsigned nr_hw_queues;
 	unsigned nr_maps;
+
+	/*
+	 * Number of reserved commands to allocate, if any.
+	 */
+	unsigned nr_reserved_cmds;
+
 	unsigned active_mode:2;
 
 	/*

[PATCH 05/24] scsi: core: Introduce scsi_host_update_can_queue()

[Bart Van Assche]

The UFS host controller driver must submit a QUERY REQUEST command to the
UFS device to query the queue depth supported by the device (bQueueDepth).
If the same infrastructure is used for allocating a QUERY REQUEST command
as for SCSI commands then .can_queue must initially be set to a low value
and increased once the queue depth is known. Hence this patch that adds a
function that modifies .can_queue.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi.c      | 26 ++++++++++++++++++++++++++
 include/scsi/scsi_host.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 53daf923ad8e..602cc32f139f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -198,6 +198,32 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 	scsi_io_completion(cmd, good_bytes);
 }
 
+/**
+ * scsi_host_update_can_queue - Modify @host->can_queue
+ *
+ * @host->__devices must be empty and I/O must have been quiesced before this
+ * function is called.
+ */
+int scsi_host_update_can_queue(struct Scsi_Host *host, int can_queue)
+{
+	struct blk_mq_tag_set prev_set;
+	int prev_can_queue, ret;
+
+	if (!list_empty(&host->__devices))
+		return -EINVAL;
+
+	prev_can_queue = host->can_queue;
+	prev_set = host->tag_set;
+	host->can_queue = can_queue;
+	ret = scsi_mq_setup_tags(host);
+	if (ret) {
+		host->can_queue = prev_can_queue;
+		return ret;
+	}
+	blk_mq_free_tag_set(&prev_set);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(scsi_host_update_can_queue);
 
 /*
  * 4096 is big enough for saturating fast SCSI LUNs.
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 2c0f5ec1046e..7b6aa36eac8a 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -830,6 +830,8 @@ extern void scsi_block_requests(struct Scsi_Host *);
 extern int scsi_host_block(struct Scsi_Host *shost);
 extern int scsi_host_unblock(struct Scsi_Host *shost, int new_state);
 
+int scsi_host_update_can_queue(struct Scsi_Host *host, int can_queue);
+
 void scsi_host_busy_iter(struct Scsi_Host *,
 			 bool (*fn)(struct scsi_cmnd *, void *), void *priv);
 

[PATCH 06/24] scsi: ufs: core: Change the type of one ufshcd_add_cmd_upiu_trace() argument

[Bart Van Assche]

Change the 'tag' argument into an LRB pointer. This patch prepares for the
removal of the hba->lrb[] array.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index c4448b94092f..3b470f564313 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -355,10 +355,11 @@ static void ufshcd_configure_wb(struct ufs_hba *hba)
 		ufshcd_wb_toggle_buf_flush(hba, true);
 }
 
-static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
+static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba,
+				      struct ufshcd_lrb *lrb,
 				      enum ufs_trace_str_t str_t)
 {
-	struct utp_upiu_req *rq = hba->lrb[tag].ucd_req_ptr;
+	struct utp_upiu_req *rq = lrb->ucd_req_ptr;
 	struct utp_upiu_header *header;
 
 	if (!trace_ufshcd_upiu_enabled())
@@ -367,7 +368,7 @@ static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
 	if (str_t == UFS_CMD_SEND)
 		header = &rq->header;
 	else
-		header = &hba->lrb[tag].ucd_rsp_ptr->header;
+		header = &lrb->ucd_rsp_ptr->header;
 
 	trace_ufshcd_upiu(hba, str_t, header, &rq->sc.cdb,
 			  UFS_TSF_CDB);
@@ -441,7 +442,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 		return;
 
 	/* trace UPIU also */
-	ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
+	ufshcd_add_cmd_upiu_trace(hba, lrbp, str_t);
 	if (!trace_ufshcd_command_enabled())
 		return;
 

[PATCH 07/24] scsi: ufs: core: Only call ufshcd_add_command_trace() for SCSI commands

[Bart Van Assche]

Instead of checking inside ufshcd_add_command_trace() whether 'cmd' points
at a SCSI command, let the caller perform that check. This patch prepares
for removing the lrbp->cmd pointer.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 3b470f564313..94cf864ac62b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -438,9 +438,6 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	int transfer_len = -1;
 
-	if (!cmd)
-		return;
-
 	/* trace UPIU also */
 	ufshcd_add_cmd_upiu_trace(hba, lrbp, str_t);
 	if (!trace_ufshcd_command_enabled())
@@ -2309,9 +2306,10 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
 	lrbp->issue_time_stamp_local_clock = local_clock();
 	lrbp->compl_time_stamp = ktime_set(0, 0);
 	lrbp->compl_time_stamp_local_clock = 0;
-	ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
-	if (lrbp->cmd)
+	if (lrbp->cmd) {
+		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
 		ufshcd_clk_scaling_start_busy(hba);
+	}
 	if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
 		ufshcd_start_monitor(hba, lrbp);
 

[PATCH 08/24] scsi: ufs: core: Change the type of one ufshcd_add_command_trace() argument

[Bart Van Assche]

Change the 'tag' argument into a SCSI command pointer. This patch prepares
for the removal of the hba->lrb[] array.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 94cf864ac62b..f87526443d8d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -425,7 +425,7 @@ static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
 				 ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3));
 }
 
-static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
+static void ufshcd_add_command_trace(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 				     enum ufs_trace_str_t str_t)
 {
 	u64 lba = 0;
@@ -433,9 +433,9 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 	u32 doorbell = 0;
 	u32 intr;
 	int hwq_id = -1;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
-	struct scsi_cmnd *cmd = lrbp->cmd;
 	struct request *rq = scsi_cmd_to_rq(cmd);
+	unsigned int tag = rq->tag;
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
 	int transfer_len = -1;
 
 	/* trace UPIU also */
@@ -453,7 +453,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 		       be32_to_cpu(lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
 		lba = scsi_get_lba(cmd);
 		if (opcode == WRITE_10)
-			group_id = lrbp->cmd->cmnd[6];
+			group_id = cmd->cmnd[6];
 	} else if (opcode == UNMAP) {
 		/*
 		 * The number of Bytes to be unmapped beginning with the lba.
@@ -2307,7 +2307,7 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
 	lrbp->compl_time_stamp = ktime_set(0, 0);
 	lrbp->compl_time_stamp_local_clock = 0;
 	if (lrbp->cmd) {
-		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
+		ufshcd_add_command_trace(hba, lrbp->cmd, UFS_CMD_SEND);
 		ufshcd_clk_scaling_start_busy(hba);
 	}
 	if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
@@ -5564,7 +5564,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 	if (cmd) {
 		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
 			ufshcd_update_monitor(hba, lrbp);
-		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);
+		ufshcd_add_command_trace(hba, cmd, UFS_CMD_COMP);
 		cmd->result = ufshcd_transfer_rsp_status(hba, lrbp, cqe);
 		ufshcd_release_scsi_cmd(hba, lrbp);
 		/* Do not touch lrbp after scsi done */

[PATCH 09/24] scsi: ufs: core: Change the type of one ufshcd_send_command() argument

[Bart Van Assche]

Change the 'task_tag' argument into an LRB pointer. This patch prepares
for the removal of the hba->lrb[] array.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f87526443d8d..883551274330 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2292,14 +2292,13 @@ static void ufshcd_update_monitor(struct ufs_hba *hba, const struct ufshcd_lrb *
 /**
  * ufshcd_send_command - Send SCSI or device management commands
  * @hba: per adapter instance
- * @task_tag: Task tag of the command
+ * @lrbp: Local reference block of SCSI command
  * @hwq: pointer to hardware queue instance
  */
-static inline
-void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
-			 struct ufs_hw_queue *hwq)
+static inline void ufshcd_send_command(struct ufs_hba *hba,
+				       struct ufshcd_lrb *lrbp,
+				       struct ufs_hw_queue *hwq)
 {
-	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
 	unsigned long flags;
 
 	lrbp->issue_time_stamp = ktime_get();
@@ -3029,7 +3028,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	if (hba->mcq_enabled)
 		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
 
-	ufshcd_send_command(hba, tag, hwq);
+	ufshcd_send_command(hba, lrbp, hwq);
 
 out:
 	if (ufs_trigger_eh(hba)) {
@@ -3263,7 +3262,7 @@ static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 	int err;
 
 	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
-	ufshcd_send_command(hba, tag, hba->dev_cmd_queue);
+	ufshcd_send_command(hba, lrbp, hba->dev_cmd_queue);
 	err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
 
 	ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : UFS_QUERY_COMP,

[PATCH 10/24] scsi: ufs: core: Only call ufshcd_should_inform_monitor() for SCSI commands

[Bart Van Assche]

ufshcd_should_inform_monitor() only returns 'true' for SCSI commands.
Instead of checking inside ufshcd_should_inform_monitor() whether its
second argument represents a SCSI command, only call this function for
SCSI commands. This patch prepares for removing the lrbp->cmd member.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 883551274330..f61ba94a8204 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2235,12 +2235,13 @@ static inline int ufshcd_monitor_opcode2dir(u8 opcode)
 		return -EINVAL;
 }
 
+/* Must only be called for SCSI commands. */
 static inline bool ufshcd_should_inform_monitor(struct ufs_hba *hba,
 						struct ufshcd_lrb *lrbp)
 {
 	const struct ufs_hba_monitor *m = &hba->monitor;
 
-	return (m->enabled && lrbp && lrbp->cmd &&
+	return (m->enabled &&
 		(!m->chunk_size || m->chunk_size == lrbp->cmd->sdb.length) &&
 		ktime_before(hba->monitor.enabled_ts, lrbp->issue_time_stamp));
 }
@@ -2308,9 +2309,9 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 	if (lrbp->cmd) {
 		ufshcd_add_command_trace(hba, lrbp->cmd, UFS_CMD_SEND);
 		ufshcd_clk_scaling_start_busy(hba);
+		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
+			ufshcd_start_monitor(hba, lrbp);
 	}
-	if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
-		ufshcd_start_monitor(hba, lrbp);
 
 	if (hba->mcq_enabled) {
 		int utrd_size = sizeof(struct utp_transfer_req_desc);

[PATCH 11/24] scsi: ufs: core: Change the monitor function argument types

[Bart Van Assche]

Pass a SCSI command pointer instead of a struct ufshcd_lrb pointer. This
patch prepares for combining the SCSI command and ufshcd_lrb data
structures into a single data structure.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f61ba94a8204..4b5734bbb12b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2237,19 +2237,20 @@ static inline int ufshcd_monitor_opcode2dir(u8 opcode)
 
 /* Must only be called for SCSI commands. */
 static inline bool ufshcd_should_inform_monitor(struct ufs_hba *hba,
-						struct ufshcd_lrb *lrbp)
+						struct scsi_cmnd *cmd)
 {
 	const struct ufs_hba_monitor *m = &hba->monitor;
+	struct request *rq = scsi_cmd_to_rq(cmd);
+	struct ufshcd_lrb *lrbp = &hba->lrb[rq->tag];
 
-	return (m->enabled &&
-		(!m->chunk_size || m->chunk_size == lrbp->cmd->sdb.length) &&
-		ktime_before(hba->monitor.enabled_ts, lrbp->issue_time_stamp));
+	return m->enabled &&
+	       (!m->chunk_size || m->chunk_size == cmd->sdb.length) &&
+	       ktime_before(hba->monitor.enabled_ts, lrbp->issue_time_stamp);
 }
 
-static void ufshcd_start_monitor(struct ufs_hba *hba,
-				 const struct ufshcd_lrb *lrbp)
+static void ufshcd_start_monitor(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
-	int dir = ufshcd_monitor_opcode2dir(*lrbp->cmd->cmnd);
+	int dir = ufshcd_monitor_opcode2dir(cmd->cmnd[0]);
 	unsigned long flags;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
@@ -2258,14 +2259,16 @@ static void ufshcd_start_monitor(struct ufs_hba *hba,
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 }
 
-static void ufshcd_update_monitor(struct ufs_hba *hba, const struct ufshcd_lrb *lrbp)
+static void ufshcd_update_monitor(struct ufs_hba *hba,
+				  const struct scsi_cmnd *cmd)
 {
-	int dir = ufshcd_monitor_opcode2dir(*lrbp->cmd->cmnd);
+	const struct request *req = scsi_cmd_to_rq(cmd);
+	struct ufshcd_lrb *lrbp = &hba->lrb[req->tag];
+	int dir = ufshcd_monitor_opcode2dir(cmd->cmnd[0]);
 	unsigned long flags;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	if (dir >= 0 && hba->monitor.nr_queued[dir] > 0) {
-		const struct request *req = scsi_cmd_to_rq(lrbp->cmd);
 		struct ufs_hba_monitor *m = &hba->monitor;
 		ktime_t now, inc, lat;
 
@@ -2309,8 +2312,8 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 	if (lrbp->cmd) {
 		ufshcd_add_command_trace(hba, lrbp->cmd, UFS_CMD_SEND);
 		ufshcd_clk_scaling_start_busy(hba);
-		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
-			ufshcd_start_monitor(hba, lrbp);
+		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp->cmd)))
+			ufshcd_start_monitor(hba, lrbp->cmd);
 	}
 
 	if (hba->mcq_enabled) {
@@ -5562,8 +5565,8 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 	lrbp->compl_time_stamp_local_clock = local_clock();
 	cmd = lrbp->cmd;
 	if (cmd) {
-		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
-			ufshcd_update_monitor(hba, lrbp);
+		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp->cmd)))
+			ufshcd_update_monitor(hba, lrbp->cmd);
 		ufshcd_add_command_trace(hba, cmd, UFS_CMD_COMP);
 		cmd->result = ufshcd_transfer_rsp_status(hba, lrbp, cqe);
 		ufshcd_release_scsi_cmd(hba, lrbp);

[PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Bart Van Assche]

Replace a tag loop with blk_mq_tagset_busy_iter(). This patch prepares
for removing the hba->lrb[] array.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 74 ++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4b5734bbb12b..a5faf5af462e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5651,6 +5651,44 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	return completed_reqs != 0;
 }
 
+static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
+{
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
+	struct scsi_device *sdev = rq->q->queuedata;
+	struct Scsi_Host *shost = sdev->host;
+	struct ufs_hba *hba = shost_priv(shost);
+	struct ufshcd_lrb *lrbp = &hba->lrb[rq->tag];
+	struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
+	unsigned long flags;
+
+	ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
+	/*
+	 * For those cmds of which the cqes are not present
+	 * in the cq, complete them explicitly.
+	 */
+	spin_lock_irqsave(&hwq->cq_lock, flags);
+	if (cmd && !test_bit(SCMD_STATE_COMPLETE, &cmd->state)) {
+		set_host_byte(cmd, DID_REQUEUE);
+		ufshcd_release_scsi_cmd(hba, lrbp);
+		scsi_done(cmd);
+	}
+	spin_unlock_irqrestore(&hwq->cq_lock, flags);
+
+	return true;
+}
+
+static bool ufshcd_mcq_compl_one(struct request *rq, void *priv)
+{
+	struct scsi_device *sdev = rq->q->queuedata;
+	struct Scsi_Host *shost = sdev->host;
+	struct ufs_hba *hba = shost_priv(shost);
+	struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
+
+	ufshcd_mcq_poll_cqe_lock(hba, hwq);
+
+	return true;
+}
+
 /**
  * ufshcd_mcq_compl_pending_transfer - MCQ mode function. It is
  * invoked from the error handler context or ufshcd_host_reset_and_restore()
@@ -5665,38 +5703,10 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
 static void ufshcd_mcq_compl_pending_transfer(struct ufs_hba *hba,
 					      bool force_compl)
 {
-	struct ufs_hw_queue *hwq;
-	struct ufshcd_lrb *lrbp;
-	struct scsi_cmnd *cmd;
-	unsigned long flags;
-	int tag;
-
-	for (tag = 0; tag < hba->nutrs; tag++) {
-		lrbp = &hba->lrb[tag];
-		cmd = lrbp->cmd;
-		if (!ufshcd_cmd_inflight(cmd) ||
-		    test_bit(SCMD_STATE_COMPLETE, &cmd->state))
-			continue;
-
-		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
-
-		if (force_compl) {
-			ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
-			/*
-			 * For those cmds of which the cqes are not present
-			 * in the cq, complete them explicitly.
-			 */
-			spin_lock_irqsave(&hwq->cq_lock, flags);
-			if (cmd && !test_bit(SCMD_STATE_COMPLETE, &cmd->state)) {
-				set_host_byte(cmd, DID_REQUEUE);
-				ufshcd_release_scsi_cmd(hba, lrbp);
-				scsi_done(cmd);
-			}
-			spin_unlock_irqrestore(&hwq->cq_lock, flags);
-		} else {
-			ufshcd_mcq_poll_cqe_lock(hba, hwq);
-		}
-	}
+	blk_mq_tagset_busy_iter(&hba->host->tag_set,
+				force_compl ? ufshcd_mcq_force_compl_one :
+					      ufshcd_mcq_compl_one,
+				NULL);
 }
 
 /**

[PATCH 13/24] scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()

[Bart Van Assche]

Merge the MCQ mode and legacy mode loops into a single loop. This patch
prepares for optimizing the hot path by removing the direct hba->lrb[]
accesses from ufshcd_eh_device_reset_handler().

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 82 +++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 46 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a5faf5af462e..4728cae130a7 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7428,6 +7428,35 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 	return err ? : result;
 }
 
+static bool ufshcd_clear_lu_cmds(struct request *req, void *priv)
+{
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
+	struct scsi_device *sdev = cmd->device;
+	struct Scsi_Host *shost = sdev->host;
+	struct ufs_hba *hba = shost_priv(shost);
+	const u64 lun = *(u64 *)priv;
+	const u32 tag = req->tag;
+
+	if (sdev->lun != lun)
+		return true;
+
+	if (ufshcd_clear_cmd(hba, tag) < 0) {
+		dev_err(hba->dev, "%s: failed to clear request %d\n", __func__,
+			tag);
+		return true;
+	}
+
+	if (hba->mcq_enabled) {
+		struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, req);
+
+		ufshcd_mcq_poll_cqe_lock(hba, hwq);
+		return true;
+	}
+
+	ufshcd_compl_one_cqe(hba, tag, NULL);
+	return true;
+}
+
 /**
  * ufshcd_eh_device_reset_handler() - Reset a single logical unit.
  * @cmd: SCSI command pointer
@@ -7436,12 +7465,8 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
  */
 static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
 {
-	unsigned long flags, pending_reqs = 0, not_cleared = 0;
 	struct Scsi_Host *host;
 	struct ufs_hba *hba;
-	struct ufs_hw_queue *hwq;
-	struct ufshcd_lrb *lrbp;
-	u32 pos, not_cleared_mask = 0;
 	int err;
 	u8 resp = 0xF, lun;
 
@@ -7450,50 +7475,15 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
 
 	lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
 	err = ufshcd_issue_tm_cmd(hba, lun, 0, UFS_LOGICAL_RESET, &resp);
-	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-		if (!err)
-			err = resp;
-		goto out;
-	}
-
-	if (hba->mcq_enabled) {
-		for (pos = 0; pos < hba->nutrs; pos++) {
-			lrbp = &hba->lrb[pos];
-			if (ufshcd_cmd_inflight(lrbp->cmd) &&
-			    lrbp->lun == lun) {
-				ufshcd_clear_cmd(hba, pos);
-				hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(lrbp->cmd));
-				ufshcd_mcq_poll_cqe_lock(hba, hwq);
-			}
-		}
-		err = 0;
-		goto out;
-	}
-
-	/* clear the commands that were pending for corresponding LUN */
-	spin_lock_irqsave(&hba->outstanding_lock, flags);
-	for_each_set_bit(pos, &hba->outstanding_reqs, hba->nutrs)
-		if (hba->lrb[pos].lun == lun)
-			__set_bit(pos, &pending_reqs);
-	hba->outstanding_reqs &= ~pending_reqs;
-	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
-
-	for_each_set_bit(pos, &pending_reqs, hba->nutrs) {
-		if (ufshcd_clear_cmd(hba, pos) < 0) {
-			spin_lock_irqsave(&hba->outstanding_lock, flags);
-			not_cleared = 1U << pos &
-				ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-			hba->outstanding_reqs |= not_cleared;
-			not_cleared_mask |= not_cleared;
-			spin_unlock_irqrestore(&hba->outstanding_lock, flags);
-
-			dev_err(hba->dev, "%s: failed to clear request %d\n",
-				__func__, pos);
-		}
+	if (err) {
+	} else if (resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+		err = resp;
+	} else {
+		/* clear the commands that were pending for corresponding LUN */
+		blk_mq_tagset_busy_iter(&hba->host->tag_set,
+				ufshcd_clear_lu_cmds, &cmd->device->lun);
 	}
-	__ufshcd_transfer_req_compl(hba, pending_reqs & ~not_cleared_mask);
 
-out:
 	hba->req_abort_count = 0;
 	ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, (u32)err);
 	if (!err) {

[PATCH 14/24] scsi: ufs: core: Cache the DMA buffer sizes

[Bart Van Assche]

Prepare for supporting DMA buffer reallocation. Caching the DMA buffer
sizes is essential because a later patch will modify hba->nutrs between
the ufshcd_memory_alloc() and the ufshcd_memory_free() calls.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 62 ++++++++++++++++++++-------------------
 include/ufs/ufshcd.h      |  5 ++++
 2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4728cae130a7..26aa07712507 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3857,14 +3857,10 @@ static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
  */
 static int ufshcd_memory_alloc(struct ufs_hba *hba)
 {
-	size_t utmrdl_size, utrdl_size, ucdl_size;
-
 	/* Allocate memory for UTP command descriptors */
-	ucdl_size = ufshcd_get_ucd_size(hba) * hba->nutrs;
-	hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
-						  ucdl_size,
-						  &hba->ucdl_dma_addr,
-						  GFP_KERNEL);
+	hba->ucdl_size = ufshcd_get_ucd_size(hba) * hba->nutrs;
+	hba->ucdl_base_addr = dmam_alloc_coherent(
+		hba->dev, hba->ucdl_size, &hba->ucdl_dma_addr, GFP_KERNEL);
 
 	/*
 	 * UFSHCI requires UTP command descriptor to be 128 byte aligned.
@@ -3880,11 +3876,9 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 	 * Allocate memory for UTP Transfer descriptors
 	 * UFSHCI requires 1KB alignment of UTRD
 	 */
-	utrdl_size = (sizeof(struct utp_transfer_req_desc) * hba->nutrs);
-	hba->utrdl_base_addr = dmam_alloc_coherent(hba->dev,
-						   utrdl_size,
-						   &hba->utrdl_dma_addr,
-						   GFP_KERNEL);
+	hba->utrdl_size = sizeof(struct utp_transfer_req_desc) * hba->nutrs;
+	hba->utrdl_base_addr = dmam_alloc_coherent(
+		hba->dev, hba->utrdl_size, &hba->utrdl_dma_addr, GFP_KERNEL);
 	if (!hba->utrdl_base_addr ||
 	    WARN_ON(hba->utrdl_dma_addr & (SZ_1K - 1))) {
 		dev_err(hba->dev,
@@ -3896,7 +3890,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 	 * Skip utmrdl allocation; it may have been
 	 * allocated during first pass and not released during
 	 * MCQ memory allocation.
-	 * See ufshcd_release_sdb_queue() and ufshcd_config_mcq()
+	 * See ufshcd_memory_free() and ufshcd_config_mcq()
 	 */
 	if (hba->utmrdl_base_addr)
 		goto skip_utmrdl;
@@ -3904,11 +3898,9 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 	 * Allocate memory for UTP Task Management descriptors
 	 * UFSHCI requires 1KB alignment of UTMRD
 	 */
-	utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
-	hba->utmrdl_base_addr = dmam_alloc_coherent(hba->dev,
-						    utmrdl_size,
-						    &hba->utmrdl_dma_addr,
-						    GFP_KERNEL);
+	hba->utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
+	hba->utmrdl_base_addr = dmam_alloc_coherent(
+		hba->dev, hba->utmrdl_size, &hba->utmrdl_dma_addr, GFP_KERNEL);
 	if (!hba->utmrdl_base_addr ||
 	    WARN_ON(hba->utmrdl_dma_addr & (SZ_1K - 1))) {
 		dev_err(hba->dev,
@@ -8705,20 +8697,30 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
 	return ret;
 }
 
-/* SDB - Single Doorbell */
-static void ufshcd_release_sdb_queue(struct ufs_hba *hba, int nutrs)
+/*
+ * Free the memory allocated by ufshcd_memory_alloc() except the utmrdl DMA
+ * memory.
+ */
+static void ufshcd_memory_free(struct ufs_hba *hba)
 {
-	size_t ucdl_size, utrdl_size;
-
-	ucdl_size = ufshcd_get_ucd_size(hba) * nutrs;
-	dmam_free_coherent(hba->dev, ucdl_size, hba->ucdl_base_addr,
-			   hba->ucdl_dma_addr);
+	if (hba->ucdl_base_addr) {
+		dmam_free_coherent(hba->dev, hba->ucdl_size,
+				   hba->ucdl_base_addr, hba->ucdl_dma_addr);
+		hba->ucdl_base_addr = NULL;
+		hba->ucdl_dma_addr = 0;
+	}
 
-	utrdl_size = sizeof(struct utp_transfer_req_desc) * nutrs;
-	dmam_free_coherent(hba->dev, utrdl_size, hba->utrdl_base_addr,
-			   hba->utrdl_dma_addr);
+	if (hba->utrdl_base_addr) {
+		dmam_free_coherent(hba->dev, hba->utrdl_size,
+				   hba->utrdl_base_addr, hba->utrdl_dma_addr);
+		hba->utrdl_base_addr = NULL;
+		hba->utrdl_dma_addr = 0;
+	}
 
-	devm_kfree(hba->dev, hba->lrb);
+	if (hba->lrb) {
+		devm_kfree(hba->dev, hba->lrb);
+		hba->lrb = NULL;
+	}
 }
 
 static int ufshcd_alloc_mcq(struct ufs_hba *hba)
@@ -8740,7 +8742,7 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
 	 * Number of supported tags in MCQ mode may be larger than SDB mode.
 	 */
 	if (hba->nutrs != old_nutrs) {
-		ufshcd_release_sdb_queue(hba, old_nutrs);
+		ufshcd_memory_free(hba);
 		ret = ufshcd_memory_alloc(hba);
 		if (ret)
 			goto err;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index e928ed0265ff..656c9b668fcc 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -847,6 +847,9 @@ enum ufshcd_mcq_opr {
 /**
  * struct ufs_hba - per adapter private structure
  * @mmio_base: UFSHCI base register address
+ * @ucdl_size: Size of UFS Command Descriptor buffer
+ * @utrdl_size: Size of UTP Transfer Request Descriptor buffer
+ * @utmrdl_size: Size of UTP Task Management Descriptor buffer
  * @ucdl_base_addr: UFS Command Descriptor base address
  * @utrdl_base_addr: UTP Transfer Request Descriptor base address
  * @utmrdl_base_addr: UTP Task Management Descriptor base address
@@ -975,6 +978,8 @@ enum ufshcd_mcq_opr {
 struct ufs_hba {
 	void __iomem *mmio_base;
 
+	u32 ucdl_size, utrdl_size, utmrdl_size;
+
 	/* Virtual memory reference */
 	struct utp_transfer_cmd_desc *ucdl_base_addr;
 	struct utp_transfer_req_desc *utrdl_base_addr;

[PATCH 15/24] scsi: ufs: core: Add an argument to ufshcd_mcq_decide_queue_depth()

[Bart Van Assche]

Prepare for allocating SCSI commands in two steps by making the UFS device
queue depth an argument of ufshcd_mcq_decide_queue_depth().

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c     | 14 ++++++++------
 drivers/ufs/core/ufshcd-priv.h |  2 +-
 drivers/ufs/core/ufshcd.c      |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 240ce135bbfb..cec21bb50cec 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -132,6 +132,7 @@ EXPORT_SYMBOL_GPL(ufshcd_mcq_queue_cfg_addr);
 /**
  * ufshcd_mcq_decide_queue_depth - decide the queue depth
  * @hba: per adapter instance
+ * @ufs_dev_qd: maximum queue depth supported by the UFS device
  *
  * Return: queue-depth on success, non-zero on error
  *
@@ -140,7 +141,7 @@ EXPORT_SYMBOL_GPL(ufshcd_mcq_queue_cfg_addr);
  * Calculates and adjusts the queue depth based on the depth
  * supported by the HC and ufs device.
  */
-int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba)
+int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba, u32 ufs_dev_qd)
 {
 	int mac;
 
@@ -160,13 +161,14 @@ int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba)
 	if (mac < 0)
 		goto err;
 
-	WARN_ON_ONCE(!hba->dev_info.bqueuedepth);
 	/*
-	 * max. value of bqueuedepth = 256, mac is host dependent.
-	 * It is mandatory for UFS device to define bQueueDepth if
-	 * shared queuing architecture is enabled.
+	 * According to the UFS standard, the UFS device queue depth
+	 * (bQueueDepth) must be in the range 1..255 if the shared queueing
+	 * architecture is supported. bQueueDepth is zero if the shared queueing
+	 * architecture is not supported.
 	 */
-	return min_t(int, mac, hba->dev_info.bqueuedepth);
+	WARN_ON_ONCE(!ufs_dev_qd);
+	return min(mac, ufs_dev_qd);
 
 err:
 	dev_err(hba->dev, "Failed to get mac, err=%d\n", mac);
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index d0a2c963a27d..3cf06fc708b4 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -65,7 +65,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 			  struct cq_entry *cqe);
 int ufshcd_mcq_init(struct ufs_hba *hba);
 void ufshcd_mcq_disable(struct ufs_hba *hba);
-int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba);
+int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba, u32 ufs_dev_qd);
 int ufshcd_mcq_memory_alloc(struct ufs_hba *hba);
 struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba,
 					   struct request *req);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 26aa07712507..99cc7c49619f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8728,7 +8728,7 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
 	int ret;
 	int old_nutrs = hba->nutrs;
 
-	ret = ufshcd_mcq_decide_queue_depth(hba);
+	ret = ufshcd_mcq_decide_queue_depth(hba, hba->dev_info.bqueuedepth);
 	if (ret < 0)
 		return ret;
 

[PATCH 16/24] scsi: ufs: core: Add an argument to ufshcd_alloc_mcq()

[Bart Van Assche]

Prepare for allocating SCSI commands in two steps by making the UFS device
queue depth an argument of ufshcd_alloc_mcq().

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 99cc7c49619f..acbf173a3732 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8723,12 +8723,12 @@ static void ufshcd_memory_free(struct ufs_hba *hba)
 	}
 }
 
-static int ufshcd_alloc_mcq(struct ufs_hba *hba)
+static int ufshcd_alloc_mcq(struct ufs_hba *hba, u32 ufs_dev_qd)
 {
 	int ret;
 	int old_nutrs = hba->nutrs;
 
-	ret = ufshcd_mcq_decide_queue_depth(hba, hba->dev_info.bqueuedepth);
+	ret = ufshcd_mcq_decide_queue_depth(hba, ufs_dev_qd);
 	if (ret < 0)
 		return ret;
 
@@ -10422,7 +10422,7 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
 
 	if (is_mcq_supported(hba)) {
 		ufshcd_mcq_enable(hba);
-		err = ufshcd_alloc_mcq(hba);
+		err = ufshcd_alloc_mcq(hba, hba->dev_info.bqueuedepth);
 		if (!err) {
 			ufshcd_config_mcq(hba);
 		} else {

[PATCH 17/24] scsi: ufs: core: Call ufshcd_mcq_init() once

[Bart Van Assche]

Make sure that ufshcd_mcq_init() is called once even if ufshcd_alloc_mcq()
is called twice.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index acbf173a3732..6dcac4143f4f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8733,9 +8733,16 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba, u32 ufs_dev_qd)
 		return ret;
 
 	hba->nutrs = ret;
-	ret = ufshcd_mcq_init(hba);
-	if (ret)
-		goto err;
+	if (hba->host->nr_hw_queues == 0) {
+		/*
+		 * ufshcd_mcq_init() is independent of hba->nutrs. Hence, only
+		 * call ufshcd_mcq_init() the first time ufshcd_alloc_mcq() is
+		 * called.
+		 */
+		ret = ufshcd_mcq_init(hba);
+		if (ret)
+			goto err;
+	}
 
 	/*
 	 * Previously allocated memory for nutrs may not be enough in MCQ mode.

[PATCH 18/24] scsi: ufs: core: Allocate the SCSI host earlier

[Bart Van Assche]

Call ufshcd_add_scsi_host() before any UPIU commands are sent to the UFS
device. This patch prepares for letting ufshcd_add_scsi_host() allocate
memory for both SCSI and UPIU commands.

Initially, allocate 32 commands (SCSI + reserved). In ufshcd_alloc_mcq(),
increase the SCSI host queue depth if necessary.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 6dcac4143f4f..7cbc7a3cf2db 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8941,6 +8941,32 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 	ktime_t probe_start;
 	int ret;
 
+	if (is_mcq_supported(hba)) {
+		ufshcd_mcq_enable(hba);
+		ret = ufshcd_alloc_mcq(hba, hba->dev_info.bqueuedepth);
+		if (ret == 0) {
+			pr_info("%s: hba->nutrs = %d\n", __func__, hba->nutrs);
+			ufshcd_config_mcq(hba);
+			ret = scsi_host_update_can_queue(
+				hba->host, hba->nutrs - UFSHCD_NUM_RESERVED);
+			if (ret)
+				goto out;
+		} else {
+			/* Continue with SDB mode */
+			ufshcd_mcq_disable(hba);
+			use_mcq_mode = false;
+			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
+				ret);
+		}
+	}
+	if (!is_mcq_supported(hba) && !hba->lsdb_sup) {
+		dev_err(hba->dev,
+			"%s: failed to initialize (legacy doorbell mode not supported)\n",
+			__func__);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	down(&hba->host_sem);
 	/* Initialize hba, detect and initialize UFS device */
 	probe_start = ktime_get();
@@ -10427,9 +10453,12 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
 {
 	int err;
 
+	WARN_ON_ONCE(!hba->host->can_queue);
+	WARN_ON_ONCE(!hba->host->cmd_per_lun);
+
 	if (is_mcq_supported(hba)) {
 		ufshcd_mcq_enable(hba);
-		err = ufshcd_alloc_mcq(hba, hba->dev_info.bqueuedepth);
+		err = ufshcd_alloc_mcq(hba, 32);
 		if (!err) {
 			ufshcd_config_mcq(hba);
 		} else {
@@ -10667,6 +10696,10 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 			    FIELD_PREP(UFSHCI_AHIBERN8_SCALE_MASK, 3);
 	}
 
+	err = ufshcd_add_scsi_host(hba);
+	if (err)
+		goto out_disable;
+
 	/* Hold auto suspend until async scan completes */
 	pm_runtime_get_sync(dev);
 
@@ -10717,10 +10750,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	if (err)
 		goto out_disable;
 
-	err = ufshcd_add_scsi_host(hba);
-	if (err)
-		goto out_disable;
-
 	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
 

[PATCH 19/24] scsi: ufs: core: Call ufshcd_init_lrb() later

[Bart Van Assche]

Call ufshcd_init_lrb() from inside ufshcd_setup_dev_cmd() instead of
ufshcd_host_memory_configure(). This patch prepares for calling
ufshcd_host_memory_configure() before the information is available
that is required to call ufshcd_setup_dev_cmd().

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 53 ++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7cbc7a3cf2db..df90163939d0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2868,8 +2868,32 @@ static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
 }
 
-static void __ufshcd_setup_cmd(struct ufshcd_lrb *lrbp, struct scsi_cmnd *cmd, u8 lun, int tag)
+static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
+{
+	struct utp_transfer_cmd_desc *cmd_descp =
+		(void *)hba->ucdl_base_addr + i * ufshcd_get_ucd_size(hba);
+	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
+	dma_addr_t cmd_desc_element_addr =
+		hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
+	u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
+	u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
+
+	lrb->utr_descriptor_ptr = utrdlp + i;
+	lrb->utrd_dma_addr =
+		hba->utrdl_dma_addr + i * sizeof(struct utp_transfer_req_desc);
+	lrb->ucd_req_ptr = (struct utp_upiu_req *)cmd_descp->command_upiu;
+	lrb->ucd_req_dma_addr = cmd_desc_element_addr;
+	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)cmd_descp->response_upiu;
+	lrb->ucd_rsp_dma_addr = cmd_desc_element_addr + response_offset;
+	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)cmd_descp->prd_table;
+	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
+}
+
+static void __ufshcd_setup_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
+			       struct scsi_cmnd *cmd, u8 lun, int tag)
 {
+	ufshcd_init_lrb(hba, lrbp, tag);
+
 	memset(lrbp->ucd_req_ptr, 0, sizeof(*lrbp->ucd_req_ptr));
 
 	lrbp->cmd = cmd;
@@ -2881,7 +2905,7 @@ static void __ufshcd_setup_cmd(struct ufshcd_lrb *lrbp, struct scsi_cmnd *cmd, u
 static void ufshcd_setup_scsi_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 				  struct scsi_cmnd *cmd, u8 lun, int tag)
 {
-	__ufshcd_setup_cmd(lrbp, cmd, lun, tag);
+	__ufshcd_setup_cmd(hba, lrbp, cmd, lun, tag);
 	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba);
 	lrbp->req_abort_skip = false;
 
@@ -2936,27 +2960,6 @@ static void ufshcd_map_queues(struct Scsi_Host *shost)
 	}
 }
 
-static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
-{
-	struct utp_transfer_cmd_desc *cmd_descp = (void *)hba->ucdl_base_addr +
-		i * ufshcd_get_ucd_size(hba);
-	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
-	dma_addr_t cmd_desc_element_addr = hba->ucdl_dma_addr +
-		i * ufshcd_get_ucd_size(hba);
-	u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
-	u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
-
-	lrb->utr_descriptor_ptr = utrdlp + i;
-	lrb->utrd_dma_addr = hba->utrdl_dma_addr +
-		i * sizeof(struct utp_transfer_req_desc);
-	lrb->ucd_req_ptr = (struct utp_upiu_req *)cmd_descp->command_upiu;
-	lrb->ucd_req_dma_addr = cmd_desc_element_addr;
-	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)cmd_descp->response_upiu;
-	lrb->ucd_rsp_dma_addr = cmd_desc_element_addr + response_offset;
-	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)cmd_descp->prd_table;
-	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
-}
-
 /**
  * ufshcd_queuecommand - main entry point for SCSI requests
  * @host: SCSI host pointer
@@ -3049,7 +3052,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 			     enum dev_cmd_type cmd_type, u8 lun, int tag)
 {
-	__ufshcd_setup_cmd(lrbp, NULL, lun, tag);
+	__ufshcd_setup_cmd(hba, lrbp, NULL, lun, tag);
 	lrbp->intr_cmd = true; /* No interrupt aggregation */
 	hba->dev_cmd.type = cmd_type;
 }
@@ -3978,8 +3981,6 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
 			utrdlp[i].response_upiu_length =
 				cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
 		}
-
-		ufshcd_init_lrb(hba, &hba->lrb[i], i);
 	}
 }
 

[PATCH 20/24] scsi: ufs: core: Use hba->reserved_slot

[Bart Van Assche]

Use hba->reserved_slot instead of open-coding it.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index cec21bb50cec..3c9d308d84e0 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -547,7 +547,7 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 	if (hba->quirks & UFSHCD_QUIRK_MCQ_BROKEN_RTC)
 		return -ETIMEDOUT;
 
-	if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) {
+	if (task_tag != hba->reserved_slot) {
 		if (!cmd)
 			return -EINVAL;
 		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));

[PATCH 21/24] scsi: ufs: core: Allocate the reserved slot as a reserved request

[Bart Van Assche]

Instead of letting the SCSI core allocate hba->nutrs - 1 commands, let
the SCSI core allocate hba->nutrs commands, set the number of reserved
tags to 1 and use the reserved tag for device management commands. This
patch changes the 'reserved slot' from hba->nutrs - 1 into 0 because
the block layer reserves the smallest tags for reserved commands.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index df90163939d0..0adc0f879196 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2417,7 +2417,7 @@ static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
 	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS_SDB) + 1;
 	hba->nutmrs =
 	((hba->capabilities & MASK_TASK_MANAGEMENT_REQUEST_SLOTS) >> 16) + 1;
-	hba->reserved_slot = hba->nutrs - 1;
+	hba->reserved_slot = 0;
 
 	hba->nortt = FIELD_GET(MASK_NUMBER_OUTSTANDING_RTT, hba->capabilities) + 1;
 
@@ -8762,7 +8762,8 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba, u32 ufs_dev_qd)
 		goto err;
 
 	hba->host->can_queue = hba->nutrs - UFSHCD_NUM_RESERVED;
-	hba->reserved_slot = hba->nutrs - UFSHCD_NUM_RESERVED;
+	hba->host->nr_reserved_cmds = UFSHCD_NUM_RESERVED;
+	hba->reserved_slot = 0;
 
 	return 0;
 err:
@@ -10593,6 +10594,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	host->can_queue = hba->nutrs - UFSHCD_NUM_RESERVED;
 	host->cmd_per_lun = hba->nutrs - UFSHCD_NUM_RESERVED;
+	host->nr_reserved_cmds = UFSHCD_NUM_RESERVED;
 	host->max_id = UFSHCD_MAX_ID;
 	host->max_lun = UFS_MAX_LUNS;
 	host->max_channel = UFSHCD_MAX_CHANNEL;

[PATCH 22/24] scsi: ufs: core: Do not clear driver-private command data

[Bart Van Assche]

Tell the SCSI core to skip the memset() call that clears driver-private
data because __ufshcd_setup_cmd() performs all necessary initialization.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0adc0f879196..615c943ce9f1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2960,6 +2960,15 @@ static void ufshcd_map_queues(struct Scsi_Host *shost)
 	}
 }
 
+/*
+ * The only purpose of this function is to make the SCSI core skip the memset()
+ * call for the private command data.
+ */
+static int ufshcd_init_cmd_priv(struct Scsi_Host *host, struct scsi_cmnd *cmd)
+{
+	return 0;
+}
+
 /**
  * ufshcd_queuecommand - main entry point for SCSI requests
  * @host: SCSI host pointer
@@ -9031,6 +9040,7 @@ static const struct scsi_host_template ufshcd_driver_template = {
 	.name			= UFSHCD,
 	.proc_name		= UFSHCD,
 	.map_queues		= ufshcd_map_queues,
+	.init_cmd_priv		= ufshcd_init_cmd_priv,
 	.queuecommand		= ufshcd_queuecommand,
 	.mq_poll		= ufshcd_poll,
 	.sdev_init		= ufshcd_sdev_init,

[PATCH 23/24] scsi: ufs: core: Optimize the hot path

[Bart Van Assche]

Convert the cmd->lrbp and lrbp->cmd memory loads into pointer offset
calculations.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufs-mcq.c       |  15 +-
 drivers/ufs/core/ufshcd-crypto.h |  18 ++-
 drivers/ufs/core/ufshcd-priv.h   |  25 +++-
 drivers/ufs/core/ufshcd.c        | 243 ++++++++++++++++---------------
 include/ufs/ufshcd.h             |   5 -
 5 files changed, 167 insertions(+), 139 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 3c9d308d84e0..57d09ff41113 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -537,8 +537,8 @@ static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
  */
 int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 {
-	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
-	struct scsi_cmnd *cmd = lrbp->cmd;
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, task_tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	struct ufs_hw_queue *hwq;
 	void __iomem *reg, *opr_sqd_base;
 	u32 nexus, id, val;
@@ -623,7 +623,8 @@ static void ufshcd_mcq_nullify_sqe(struct utp_transfer_req_desc *utrd)
 static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
 				  struct ufs_hw_queue *hwq, int task_tag)
 {
-	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, task_tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	struct utp_transfer_req_desc *utrd;
 	__le64  cmd_desc_base_addr;
 	bool ret = false;
@@ -674,12 +675,12 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 	struct Scsi_Host *host = cmd->device->host;
 	struct ufs_hba *hba = shost_priv(host);
 	int tag = scsi_cmd_to_rq(cmd)->tag;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	struct ufs_hw_queue *hwq;
 	unsigned long flags;
 	int err;
 
-	if (!ufshcd_cmd_inflight(lrbp->cmd)) {
+	if (!ufshcd_cmd_inflight(cmd)) {
 		dev_err(hba->dev,
 			"%s: skip abort. cmd at tag %d already completed.\n",
 			__func__, tag);
@@ -718,8 +719,8 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 	}
 
 	spin_lock_irqsave(&hwq->cq_lock, flags);
-	if (ufshcd_cmd_inflight(lrbp->cmd))
-		ufshcd_release_scsi_cmd(hba, lrbp);
+	if (ufshcd_cmd_inflight(cmd))
+		ufshcd_release_scsi_cmd(hba, cmd);
 	spin_unlock_irqrestore(&hwq->cq_lock, flags);
 
 	return SUCCESS;
diff --git a/drivers/ufs/core/ufshcd-crypto.h b/drivers/ufs/core/ufshcd-crypto.h
index 89bb97c14c15..c148a5194378 100644
--- a/drivers/ufs/core/ufshcd-crypto.h
+++ b/drivers/ufs/core/ufshcd-crypto.h
@@ -38,10 +38,10 @@ ufshcd_prepare_req_desc_hdr_crypto(struct ufshcd_lrb *lrbp,
 }
 
 static inline int ufshcd_crypto_fill_prdt(struct ufs_hba *hba,
-					  struct ufshcd_lrb *lrbp)
+					  struct scsi_cmnd *cmd)
 {
-	struct scsi_cmnd *cmd = lrbp->cmd;
 	const struct bio_crypt_ctx *crypt_ctx = scsi_cmd_to_rq(cmd)->crypt_ctx;
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 
 	if (crypt_ctx && hba->vops && hba->vops->fill_crypto_prdt)
 		return hba->vops->fill_crypto_prdt(hba, crypt_ctx,
@@ -51,17 +51,19 @@ static inline int ufshcd_crypto_fill_prdt(struct ufs_hba *hba,
 }
 
 static inline void ufshcd_crypto_clear_prdt(struct ufs_hba *hba,
-					    struct ufshcd_lrb *lrbp)
+					    struct scsi_cmnd *cmd)
 {
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
 	if (!(hba->quirks & UFSHCD_QUIRK_KEYS_IN_PRDT))
 		return;
 
-	if (!(scsi_cmd_to_rq(lrbp->cmd)->crypt_ctx))
+	if (!(scsi_cmd_to_rq(cmd)->crypt_ctx))
 		return;
 
 	/* Zeroize the PRDT because it can contain cryptographic keys. */
 	memzero_explicit(lrbp->ucd_prdt_ptr,
-			 ufshcd_sg_entry_size(hba) * scsi_sg_count(lrbp->cmd));
+			 ufshcd_sg_entry_size(hba) * scsi_sg_count(cmd));
 }
 
 bool ufshcd_crypto_enable(struct ufs_hba *hba);
@@ -82,13 +84,15 @@ ufshcd_prepare_req_desc_hdr_crypto(struct ufshcd_lrb *lrbp,
 				   struct request_desc_header *h) { }
 
 static inline int ufshcd_crypto_fill_prdt(struct ufs_hba *hba,
-					  struct ufshcd_lrb *lrbp)
+					  struct scsi_cmnd *cmd)
 {
 	return 0;
 }
 
 static inline void ufshcd_crypto_clear_prdt(struct ufs_hba *hba,
-					    struct ufshcd_lrb *lrbp) { }
+					    struct scsi_cmnd *cmd)
+{
+}
 
 static inline bool ufshcd_crypto_enable(struct ufs_hba *hba)
 {
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 3cf06fc708b4..e5a470821669 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -75,8 +75,7 @@ bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd);
 int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag);
 int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
 int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
-void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
-			     struct ufshcd_lrb *lrbp);
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd);
 
 #define SD_ASCII_STD true
 #define SD_RAW false
@@ -361,6 +360,28 @@ static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info, u8
 	return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info->max_lu_supported);
 }
 
+/*
+ * Convert a block layer tag into a SCSI command pointer. This function is
+ * called once per I/O completion path and is also called from error paths.
+ */
+static inline struct scsi_cmnd *ufshcd_tag_to_cmd(struct ufs_hba *hba, u32 tag)
+{
+	struct blk_mq_tags *tags = hba->host->tag_set.tags[0];
+	struct request *rq;
+
+	/*
+	 * Use .static_rqs[] for reserved commands because blk_mq_get_tag()
+	 * is not called for reserved commands by the UFS driver.
+	 */
+	rq = tag < UFSHCD_NUM_RESERVED ? tags->static_rqs[tag] :
+					 blk_mq_tag_to_rq(tags, tag);
+
+	if (WARN_ON_ONCE(!rq))
+		return NULL;
+
+	return blk_mq_rq_to_pdu(rq);
+}
+
 static inline void ufshcd_inc_sq_tail(struct ufs_hw_queue *q)
 	__must_hold(&q->sq_lock)
 {
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 615c943ce9f1..1b8be737e470 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -28,6 +28,7 @@
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_driver.h>
 #include <scsi/scsi_eh.h>
+#include <scsi/scsi_tcq.h>
 #include "ufshcd-priv.h"
 #include <ufs/ufs_quirks.h>
 #include <ufs/unipro.h>
@@ -435,7 +436,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	int hwq_id = -1;
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	unsigned int tag = rq->tag;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int transfer_len = -1;
 
 	/* trace UPIU also */
@@ -546,14 +547,13 @@ static void ufshcd_print_evt_hist(struct ufs_hba *hba)
 	ufshcd_vops_dbg_register_dump(hba);
 }
 
-static
-void ufshcd_print_tr(struct ufs_hba *hba, int tag, bool pr_prdt)
+static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+			    bool pr_prdt)
 {
-	const struct ufshcd_lrb *lrbp;
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	const int tag = lrbp->task_tag;
 	int prdt_length;
 
-	lrbp = &hba->lrb[tag];
-
 	dev_err(hba->dev, "UPIU[%d] - issue time %lld us\n",
 			tag, div_u64(lrbp->issue_time_stamp_local_clock, 1000));
 	dev_err(hba->dev, "UPIU[%d] - complete time %lld us\n",
@@ -594,7 +594,7 @@ static bool ufshcd_print_tr_iter(struct request *req, void *priv)
 	struct Scsi_Host *shost = sdev->host;
 	struct ufs_hba *hba = shost_priv(shost);
 
-	ufshcd_print_tr(hba, req->tag, *(bool *)priv);
+	ufshcd_print_tr(hba, blk_mq_rq_to_pdu(req), *(bool *)priv);
 
 	return true;
 }
@@ -2240,8 +2240,7 @@ static inline bool ufshcd_should_inform_monitor(struct ufs_hba *hba,
 						struct scsi_cmnd *cmd)
 {
 	const struct ufs_hba_monitor *m = &hba->monitor;
-	struct request *rq = scsi_cmd_to_rq(cmd);
-	struct ufshcd_lrb *lrbp = &hba->lrb[rq->tag];
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 
 	return m->enabled &&
 	       (!m->chunk_size || m->chunk_size == cmd->sdb.length) &&
@@ -2263,7 +2262,7 @@ static void ufshcd_update_monitor(struct ufs_hba *hba,
 				  const struct scsi_cmnd *cmd)
 {
 	const struct request *req = scsi_cmd_to_rq(cmd);
-	struct ufshcd_lrb *lrbp = &hba->lrb[req->tag];
+	const struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int dir = ufshcd_monitor_opcode2dir(cmd->cmnd[0]);
 	unsigned long flags;
 
@@ -2293,27 +2292,37 @@ static void ufshcd_update_monitor(struct ufs_hba *hba,
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 }
 
+/*
+ * Returns %true if @cmd represents a SCSI command that is in flight and %false
+ * if it represents a device management command.
+ */
+static bool ufshcd_is_scsi_cmd(struct scsi_cmnd *cmd)
+{
+	return blk_mq_request_started(scsi_cmd_to_rq(cmd));
+}
+
 /**
  * ufshcd_send_command - Send SCSI or device management commands
  * @hba: per adapter instance
- * @lrbp: Local reference block of SCSI command
+ * @cmd: SCSI command or device management command pointer
  * @hwq: pointer to hardware queue instance
  */
 static inline void ufshcd_send_command(struct ufs_hba *hba,
-				       struct ufshcd_lrb *lrbp,
+				       struct scsi_cmnd *cmd,
 				       struct ufs_hw_queue *hwq)
 {
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	unsigned long flags;
 
 	lrbp->issue_time_stamp = ktime_get();
 	lrbp->issue_time_stamp_local_clock = local_clock();
 	lrbp->compl_time_stamp = ktime_set(0, 0);
 	lrbp->compl_time_stamp_local_clock = 0;
-	if (lrbp->cmd) {
-		ufshcd_add_command_trace(hba, lrbp->cmd, UFS_CMD_SEND);
+	if (ufshcd_is_scsi_cmd(cmd)) {
+		ufshcd_add_command_trace(hba, cmd, UFS_CMD_SEND);
 		ufshcd_clk_scaling_start_busy(hba);
-		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp->cmd)))
-			ufshcd_start_monitor(hba, lrbp->cmd);
+		if (unlikely(ufshcd_should_inform_monitor(hba, cmd)))
+			ufshcd_start_monitor(hba, cmd);
 	}
 
 	if (hba->mcq_enabled) {
@@ -2330,7 +2339,7 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 		spin_lock_irqsave(&hba->outstanding_lock, flags);
 		if (hba->vops && hba->vops->setup_xfer_req)
 			hba->vops->setup_xfer_req(hba, lrbp->task_tag,
-						  !!lrbp->cmd);
+						  ufshcd_is_scsi_cmd(cmd));
 		__set_bit(lrbp->task_tag, &hba->outstanding_reqs);
 		ufshcd_writel(hba, 1 << lrbp->task_tag,
 			      REG_UTP_TRANSFER_REQ_DOOR_BELL);
@@ -2340,11 +2349,12 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 
 /**
  * ufshcd_copy_sense_data - Copy sense data in case of check condition
- * @lrbp: pointer to local reference block
+ * @cmd: SCSI command
  */
-static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
+static inline void ufshcd_copy_sense_data(struct scsi_cmnd *cmd)
 {
-	u8 *const sense_buffer = lrbp->cmd->sense_buffer;
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	u8 *const sense_buffer = cmd->sense_buffer;
 	u16 resp_len;
 	int len;
 
@@ -2644,13 +2654,13 @@ static void ufshcd_sgl_to_prdt(struct ufs_hba *hba, struct ufshcd_lrb *lrbp, int
 /**
  * ufshcd_map_sg - Map scatter-gather list to prdt
  * @hba: per adapter instance
- * @lrbp: pointer to local reference block
+ * @cmd: SCSI command
  *
  * Return: 0 in case of success, non-zero value in case of failure.
  */
-static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+static int ufshcd_map_sg(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
-	struct scsi_cmnd *cmd = lrbp->cmd;
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int sg_segments = scsi_dma_map(cmd);
 
 	if (sg_segments < 0)
@@ -2658,7 +2668,7 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 
 	ufshcd_sgl_to_prdt(hba, lrbp, sg_segments, scsi_sglist(cmd));
 
-	return ufshcd_crypto_fill_prdt(hba, lrbp);
+	return ufshcd_crypto_fill_prdt(hba, cmd);
 }
 
 /**
@@ -2743,13 +2753,13 @@ ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 /**
  * ufshcd_prepare_utp_scsi_cmd_upiu() - fills the utp_transfer_req_desc,
  * for scsi commands
- * @lrbp: local reference block pointer
+ * @cmd: SCSI command
  * @upiu_flags: flags
  */
-static
-void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u8 upiu_flags)
+static void ufshcd_prepare_utp_scsi_cmd_upiu(struct scsi_cmnd *cmd,
+					     u8 upiu_flags)
 {
-	struct scsi_cmnd *cmd = lrbp->cmd;
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr;
 	unsigned short cdb_len;
 
@@ -2856,20 +2866,23 @@ static int ufshcd_compose_devman_upiu(struct ufs_hba *hba,
  * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  */
-static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
-	struct request *rq = scsi_cmd_to_rq(lrbp->cmd);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	struct request *rq = scsi_cmd_to_rq(cmd);
 	unsigned int ioprio_class = IOPRIO_PRIO_CLASS(req_get_ioprio(rq));
 	u8 upiu_flags;
 
-	ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, lrbp->cmd->sc_data_direction, 0);
+	ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,
+				    cmd->sc_data_direction, 0);
 	if (ioprio_class == IOPRIO_CLASS_RT)
 		upiu_flags |= UPIU_CMD_FLAGS_CP;
-	ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
+	ufshcd_prepare_utp_scsi_cmd_upiu(cmd, upiu_flags);
 }
 
-static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
+static void ufshcd_init_lrb(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
+	const int i = scsi_cmd_to_rq(cmd)->tag;
 	struct utp_transfer_cmd_desc *cmd_descp =
 		(void *)hba->ucdl_base_addr + i * ufshcd_get_ucd_size(hba);
 	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
@@ -2877,6 +2890,7 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
 		hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
 	u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
 	u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
+	struct ufshcd_lrb *lrb = scsi_cmd_priv(cmd);
 
 	lrb->utr_descriptor_ptr = utrdlp + i;
 	lrb->utrd_dma_addr =
@@ -2889,27 +2903,30 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
 	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
 }
 
-static void __ufshcd_setup_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
-			       struct scsi_cmnd *cmd, u8 lun, int tag)
+static void __ufshcd_setup_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+			       u8 lun, int tag)
 {
-	ufshcd_init_lrb(hba, lrbp, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
+	ufshcd_init_lrb(hba, cmd);
 
 	memset(lrbp->ucd_req_ptr, 0, sizeof(*lrbp->ucd_req_ptr));
 
-	lrbp->cmd = cmd;
 	lrbp->task_tag = tag;
 	lrbp->lun = lun;
 	ufshcd_prepare_lrbp_crypto(cmd ? scsi_cmd_to_rq(cmd) : NULL, lrbp);
 }
 
-static void ufshcd_setup_scsi_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
-				  struct scsi_cmnd *cmd, u8 lun, int tag)
+static void ufshcd_setup_scsi_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+				  u8 lun, int tag)
 {
-	__ufshcd_setup_cmd(hba, lrbp, cmd, lun, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
+	__ufshcd_setup_cmd(hba, cmd, lun, tag);
 	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba);
 	lrbp->req_abort_skip = false;
 
-	ufshcd_comp_scsi_upiu(hba, lrbp);
+	ufshcd_comp_scsi_upiu(hba, cmd);
 }
 
 /**
@@ -2980,7 +2997,6 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 {
 	struct ufs_hba *hba = shost_priv(host);
 	int tag = scsi_cmd_to_rq(cmd)->tag;
-	struct ufshcd_lrb *lrbp;
 	int err = 0;
 	struct ufs_hw_queue *hwq = NULL;
 
@@ -3031,11 +3047,10 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 
 	ufshcd_hold(hba);
 
-	lrbp = &hba->lrb[tag];
+	ufshcd_setup_scsi_cmd(hba, cmd,
+			      ufshcd_scsi_to_upiu_lun(cmd->device->lun), tag);
 
-	ufshcd_setup_scsi_cmd(hba, lrbp, cmd, ufshcd_scsi_to_upiu_lun(cmd->device->lun), tag);
-
-	err = ufshcd_map_sg(hba, lrbp);
+	err = ufshcd_map_sg(hba, cmd);
 	if (err) {
 		ufshcd_release(hba);
 		goto out;
@@ -3044,7 +3059,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	if (hba->mcq_enabled)
 		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
 
-	ufshcd_send_command(hba, lrbp, hwq);
+	ufshcd_send_command(hba, cmd, hwq);
 
 out:
 	if (ufs_trigger_eh(hba)) {
@@ -3058,18 +3073,22 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	return err;
 }
 
-static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
-			     enum dev_cmd_type cmd_type, u8 lun, int tag)
+static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+				 enum dev_cmd_type cmd_type, u8 lun, int tag)
 {
-	__ufshcd_setup_cmd(hba, lrbp, NULL, lun, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
+	__ufshcd_setup_cmd(hba, cmd, lun, tag);
 	lrbp->intr_cmd = true; /* No interrupt aggregation */
 	hba->dev_cmd.type = cmd_type;
 }
 
-static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
-		struct ufshcd_lrb *lrbp, enum dev_cmd_type cmd_type, int tag)
+static int ufshcd_compose_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+				  enum dev_cmd_type cmd_type, int tag)
 {
-	ufshcd_setup_dev_cmd(hba, lrbp, cmd_type, 0, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
+	ufshcd_setup_dev_cmd(hba, cmd, cmd_type, 0, tag);
 
 	return ufshcd_compose_devman_upiu(hba, lrbp);
 }
@@ -3272,13 +3291,14 @@ static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
 	ufshcd_release(hba);
 }
 
-static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
-			  const u32 tag, int timeout)
+static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
+				const u32 tag, int timeout)
 {
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int err;
 
 	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
-	ufshcd_send_command(hba, lrbp, hba->dev_cmd_queue);
+	ufshcd_send_command(hba, cmd, hba->dev_cmd_queue);
 	err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
 
 	ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : UFS_QUERY_COMP,
@@ -3302,17 +3322,17 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
 		enum dev_cmd_type cmd_type, int timeout)
 {
 	const u32 tag = hba->reserved_slot;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, tag);
 	int err;
 
 	/* Protects use of hba->reserved_slot. */
 	lockdep_assert_held(&hba->dev_cmd.lock);
 
-	err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag);
+	err = ufshcd_compose_dev_cmd(hba, cmd, cmd_type, tag);
 	if (unlikely(err))
 		return err;
 
-	return ufshcd_issue_dev_cmd(hba, lrbp, tag, timeout);
+	return ufshcd_issue_dev_cmd(hba, cmd, tag, timeout);
 }
 
 /**
@@ -3921,14 +3941,6 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 	}
 
 skip_utmrdl:
-	/* Allocate memory for local reference block */
-	hba->lrb = devm_kcalloc(hba->dev,
-				hba->nutrs, sizeof(struct ufshcd_lrb),
-				GFP_KERNEL);
-	if (!hba->lrb) {
-		dev_err(hba->dev, "LRB Memory allocation failed\n");
-		goto out;
-	}
 	return 0;
 out:
 	return -ENOMEM;
@@ -5329,19 +5341,18 @@ static void ufshcd_sdev_destroy(struct scsi_device *sdev)
 
 /**
  * ufshcd_scsi_cmd_status - Update SCSI command result based on SCSI status
- * @lrbp: pointer to local reference block of completed command
+ * @cmd: SCSI command
  * @scsi_status: SCSI command status
  *
  * Return: value base on SCSI command status.
  */
-static inline int
-ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
+static inline int ufshcd_scsi_cmd_status(struct scsi_cmnd *cmd, int scsi_status)
 {
 	int result = 0;
 
 	switch (scsi_status) {
 	case SAM_STAT_CHECK_CONDITION:
-		ufshcd_copy_sense_data(lrbp);
+		ufshcd_copy_sense_data(cmd);
 		fallthrough;
 	case SAM_STAT_GOOD:
 		result |= DID_OK << 16 | scsi_status;
@@ -5349,7 +5360,7 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
 	case SAM_STAT_TASK_SET_FULL:
 	case SAM_STAT_BUSY:
 	case SAM_STAT_TASK_ABORTED:
-		ufshcd_copy_sense_data(lrbp);
+		ufshcd_copy_sense_data(cmd);
 		result |= scsi_status;
 		break;
 	default:
@@ -5363,15 +5374,16 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
 /**
  * ufshcd_transfer_rsp_status - Get overall status of the response
  * @hba: per adapter instance
- * @lrbp: pointer to local reference block of completed command
+ * @cmd: SCSI command
  * @cqe: pointer to the completion queue entry
  *
  * Return: result of the command to notify SCSI midlayer.
  */
-static inline int
-ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
-			   struct cq_entry *cqe)
+static inline int ufshcd_transfer_rsp_status(struct ufs_hba *hba,
+					     struct scsi_cmnd *cmd,
+					     struct cq_entry *cqe)
 {
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int result = 0;
 	int scsi_status;
 	enum utp_ocs ocs;
@@ -5385,7 +5397,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 	 * not set either flag.
 	 */
 	if (resid && !(upiu_flags & UPIU_RSP_FLAG_OVERFLOW))
-		scsi_set_resid(lrbp->cmd, resid);
+		scsi_set_resid(cmd, resid);
 
 	/* overall command status of utrd */
 	ocs = ufshcd_get_tr_ocs(lrbp, cqe);
@@ -5406,7 +5418,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 			 * to notify the SCSI midlayer of the command status
 			 */
 			scsi_status = lrbp->ucd_rsp_ptr->header.status;
-			result = ufshcd_scsi_cmd_status(lrbp, scsi_status);
+			result = ufshcd_scsi_cmd_status(cmd, scsi_status);
 
 			/*
 			 * Currently we are only supporting BKOPs exception
@@ -5469,7 +5481,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 
 	if ((host_byte(result) != DID_OK) &&
 	    (host_byte(result) != DID_REQUEUE) && !hba->silence_err_logs)
-		ufshcd_print_tr(hba, lrbp->task_tag, true);
+		ufshcd_print_tr(hba, cmd, true);
 	return result;
 }
 
@@ -5538,13 +5550,10 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
 }
 
 /* Release the resources allocated for processing a SCSI command. */
-void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
-			     struct ufshcd_lrb *lrbp)
+void ufshcd_release_scsi_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
-	struct scsi_cmnd *cmd = lrbp->cmd;
-
 	scsi_dma_unmap(cmd);
-	ufshcd_crypto_clear_prdt(hba, lrbp);
+	ufshcd_crypto_clear_prdt(hba, cmd);
 	ufshcd_release(hba);
 	ufshcd_clk_scaling_update_busy(hba);
 }
@@ -5558,20 +5567,18 @@ void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
 void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 			  struct cq_entry *cqe)
 {
-	struct ufshcd_lrb *lrbp;
-	struct scsi_cmnd *cmd;
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, task_tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	enum utp_ocs ocs;
 
-	lrbp = &hba->lrb[task_tag];
 	lrbp->compl_time_stamp = ktime_get();
 	lrbp->compl_time_stamp_local_clock = local_clock();
-	cmd = lrbp->cmd;
-	if (cmd) {
-		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp->cmd)))
-			ufshcd_update_monitor(hba, lrbp->cmd);
+	if (ufshcd_is_scsi_cmd(cmd)) {
+		if (unlikely(ufshcd_should_inform_monitor(hba, cmd)))
+			ufshcd_update_monitor(hba, cmd);
 		ufshcd_add_command_trace(hba, cmd, UFS_CMD_COMP);
-		cmd->result = ufshcd_transfer_rsp_status(hba, lrbp, cqe);
-		ufshcd_release_scsi_cmd(hba, lrbp);
+		cmd->result = ufshcd_transfer_rsp_status(hba, cmd, cqe);
+		ufshcd_release_scsi_cmd(hba, cmd);
 		/* Do not touch lrbp after scsi done */
 		scsi_done(cmd);
 	} else {
@@ -5608,7 +5615,7 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
 	int tag;
 
 	for_each_set_bit(tag, completed_reqs, hba->nutrs) {
-		struct scsi_cmnd *cmd = hba->lrb[tag].cmd;
+		struct scsi_cmnd *cmd = scsi_host_find_tag(hba->host, tag);
 
 		if (!cmd)
 			continue;
@@ -5659,7 +5666,6 @@ static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
 	struct scsi_device *sdev = rq->q->queuedata;
 	struct Scsi_Host *shost = sdev->host;
 	struct ufs_hba *hba = shost_priv(shost);
-	struct ufshcd_lrb *lrbp = &hba->lrb[rq->tag];
 	struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
 	unsigned long flags;
 
@@ -5671,7 +5677,7 @@ static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
 	spin_lock_irqsave(&hwq->cq_lock, flags);
 	if (cmd && !test_bit(SCMD_STATE_COMPLETE, &cmd->state)) {
 		set_host_byte(cmd, DID_REQUEUE);
-		ufshcd_release_scsi_cmd(hba, lrbp);
+		ufshcd_release_scsi_cmd(hba, cmd);
 		scsi_done(cmd);
 	}
 	spin_unlock_irqrestore(&hwq->cq_lock, flags);
@@ -6533,7 +6539,7 @@ static bool ufshcd_abort_one(struct request *rq, void *priv)
 
 	*ret = ufshcd_try_to_abort_task(hba, tag);
 	dev_err(hba->dev, "Aborting tag %d / CDB %#02x %s\n", tag,
-		hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1,
+		ufshcd_is_scsi_cmd(cmd) ? cmd->cmnd[0] : -1,
 		*ret ? "failed" : "succeeded");
 
 	return *ret == 0;
@@ -7224,14 +7230,15 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 					enum query_opcode desc_op)
 {
 	const u32 tag = hba->reserved_slot;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int err = 0;
 	u8 upiu_flags;
 
 	/* Protects use of hba->reserved_slot. */
 	lockdep_assert_held(&hba->dev_cmd.lock);
 
-	ufshcd_setup_dev_cmd(hba, lrbp, cmd_type, 0, tag);
+	ufshcd_setup_dev_cmd(hba, cmd, cmd_type, 0, tag);
 
 	ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, DMA_NONE, 0);
 
@@ -7256,7 +7263,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 	 * bound to fail since dev_cmd.query and dev_cmd.type were left empty.
 	 * read the response directly ignoring all errors.
 	 */
-	ufshcd_issue_dev_cmd(hba, lrbp, tag, QUERY_REQ_TIMEOUT);
+	ufshcd_issue_dev_cmd(hba, cmd, tag, QUERY_REQ_TIMEOUT);
 
 	/* just copy the upiu response as it is */
 	memcpy(rsp_upiu, lrbp->ucd_rsp_ptr, sizeof(*rsp_upiu));
@@ -7372,7 +7379,8 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 			 enum dma_data_direction dir)
 {
 	const u32 tag = hba->reserved_slot;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int err = 0;
 	int result;
 	u8 upiu_flags;
@@ -7383,7 +7391,8 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 	/* Protects use of hba->reserved_slot. */
 	ufshcd_dev_man_lock(hba);
 
-	ufshcd_setup_dev_cmd(hba, lrbp, DEV_CMD_TYPE_RPMB, UFS_UPIU_RPMB_WLUN, tag);
+	ufshcd_setup_dev_cmd(hba, cmd, DEV_CMD_TYPE_RPMB, UFS_UPIU_RPMB_WLUN,
+			     tag);
 
 	ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, DMA_NONE, ehs);
 
@@ -7400,7 +7409,7 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 
 	memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));
 
-	err = ufshcd_issue_dev_cmd(hba, lrbp, tag, ADVANCED_RPMB_REQ_TIMEOUT);
+	err = ufshcd_issue_dev_cmd(hba, cmd, tag, ADVANCED_RPMB_REQ_TIMEOUT);
 
 	if (!err) {
 		/* Just copy the upiu response as it is */
@@ -7499,11 +7508,12 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
 
 static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 {
-	struct ufshcd_lrb *lrbp;
 	int tag;
 
 	for_each_set_bit(tag, &bitmap, hba->nutrs) {
-		lrbp = &hba->lrb[tag];
+		struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, tag);
+		struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+
 		lrbp->req_abort_skip = true;
 	}
 }
@@ -7511,7 +7521,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 /**
  * ufshcd_try_to_abort_task - abort a specific task
  * @hba: Pointer to adapter instance
- * @tag: Task tag/index to be aborted
+ * @tag: Tag of the task to be aborted
  *
  * Abort the pending command in device by sending UFS_ABORT_TASK task management
  * command, and in host controller by clearing the door-bell register. There can
@@ -7523,7 +7533,8 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  */
 int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 {
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, tag);
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	int err;
 	int poll_cnt;
 	u8 resp = 0xF;
@@ -7545,7 +7556,7 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 				hba->dev,
 				"%s: cmd with tag %d not pending in the device.\n",
 				__func__, tag);
-			if (!ufshcd_cmd_inflight(lrbp->cmd)) {
+			if (!ufshcd_cmd_inflight(cmd)) {
 				dev_info(hba->dev,
 					 "%s: cmd with tag=%d completed.\n",
 					 __func__, tag);
@@ -7593,7 +7604,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	struct Scsi_Host *host = cmd->device->host;
 	struct ufs_hba *hba = shost_priv(host);
 	int tag = scsi_cmd_to_rq(cmd)->tag;
-	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	unsigned long flags;
 	int err = FAILED;
 	bool outstanding;
@@ -7628,9 +7639,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-		ufshcd_print_tr(hba, tag, true);
+		ufshcd_print_tr(hba, cmd, true);
 	} else {
-		ufshcd_print_tr(hba, tag, false);
+		ufshcd_print_tr(hba, cmd, false);
 	}
 	hba->req_abort_count++;
 
@@ -7674,7 +7685,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto release;
 	}
 
-	err = ufshcd_try_to_abort_task(hba, tag);
+	err = ufshcd_try_to_abort_task(hba, lrbp->task_tag);
 	if (err) {
 		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
 		ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
@@ -7691,7 +7702,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 
 	if (outstanding)
-		ufshcd_release_scsi_cmd(hba, lrbp);
+		ufshcd_release_scsi_cmd(hba, cmd);
 
 	err = SUCCESS;
 
@@ -8726,11 +8737,6 @@ static void ufshcd_memory_free(struct ufs_hba *hba)
 		hba->utrdl_base_addr = NULL;
 		hba->utrdl_dma_addr = 0;
 	}
-
-	if (hba->lrb) {
-		devm_kfree(hba->dev, hba->lrb);
-		hba->lrb = NULL;
-	}
 }
 
 static int ufshcd_alloc_mcq(struct ufs_hba *hba, u32 ufs_dev_qd)
@@ -9040,6 +9046,7 @@ static const struct scsi_host_template ufshcd_driver_template = {
 	.name			= UFSHCD,
 	.proc_name		= UFSHCD,
 	.map_queues		= ufshcd_map_queues,
+	.cmd_size		= sizeof(struct ufshcd_lrb),
 	.init_cmd_priv		= ufshcd_init_cmd_priv,
 	.queuecommand		= ufshcd_queuecommand,
 	.mq_poll		= ufshcd_poll,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 656c9b668fcc..3e7ee781b841 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -161,7 +161,6 @@ struct ufs_pm_lvl_states {
  * @ucd_prdt_dma_addr: PRDT dma address for debug
  * @ucd_rsp_dma_addr: UPIU response dma address for debug
  * @ucd_req_dma_addr: UPIU request dma address for debug
- * @cmd: pointer to SCSI command
  * @scsi_status: SCSI status of the command
  * @command_type: SCSI, UFS, Query.
  * @task_tag: Task tag of the command
@@ -186,7 +185,6 @@ struct ufshcd_lrb {
 	dma_addr_t ucd_rsp_dma_addr;
 	dma_addr_t ucd_prdt_dma_addr;
 
-	struct scsi_cmnd *cmd;
 	int scsi_status;
 
 	int command_type;
@@ -866,7 +864,6 @@ enum ufshcd_mcq_opr {
  * @spm_lvl: desired UFS power management level during system PM.
  * @pm_op_in_progress: whether or not a PM operation is in progress.
  * @ahit: value of Auto-Hibernate Idle Timer register.
- * @lrb: local reference block
  * @outstanding_tasks: Bits representing outstanding task requests
  * @outstanding_lock: Protects @outstanding_reqs.
  * @outstanding_reqs: Bits representing outstanding transfer requests
@@ -1009,8 +1006,6 @@ struct ufs_hba {
 	/* Auto-Hibernate Idle Timer register value */
 	u32 ahit;
 
-	struct ufshcd_lrb *lrb;
-
 	unsigned long outstanding_tasks;
 	spinlock_t outstanding_lock;
 	unsigned long outstanding_reqs;

[PATCH 24/24] scsi: ufs: core: Remove the ufshcd_lrb task_tag member

[Bart Van Assche]

Remove the ufshcd_lrb task_tag member and use scsi_cmd_to_rq(cmd)->tag
instead.

Use rq->tag instead of lrbp->task_tag.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 66 +++++++++++++++++++--------------------
 include/ufs/ufshcd.h      |  1 -
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 1b8be737e470..b7769eb190f7 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -551,7 +551,7 @@ static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 			    bool pr_prdt)
 {
 	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
-	const int tag = lrbp->task_tag;
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	int prdt_length;
 
 	dev_err(hba->dev, "UPIU[%d] - issue time %lld us\n",
@@ -2312,6 +2312,7 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 				       struct ufs_hw_queue *hwq)
 {
 	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	unsigned long flags;
 
 	lrbp->issue_time_stamp = ktime_get();
@@ -2338,11 +2339,10 @@ static inline void ufshcd_send_command(struct ufs_hba *hba,
 	} else {
 		spin_lock_irqsave(&hba->outstanding_lock, flags);
 		if (hba->vops && hba->vops->setup_xfer_req)
-			hba->vops->setup_xfer_req(hba, lrbp->task_tag,
+			hba->vops->setup_xfer_req(hba, tag,
 						  ufshcd_is_scsi_cmd(cmd));
-		__set_bit(lrbp->task_tag, &hba->outstanding_reqs);
-		ufshcd_writel(hba, 1 << lrbp->task_tag,
-			      REG_UTP_TRANSFER_REQ_DOOR_BELL);
+		__set_bit(tag, &hba->outstanding_reqs);
+		ufshcd_writel(hba, 1 << tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 		spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 	}
 }
@@ -2760,6 +2760,7 @@ static void ufshcd_prepare_utp_scsi_cmd_upiu(struct scsi_cmnd *cmd,
 					     u8 upiu_flags)
 {
 	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr;
 	unsigned short cdb_len;
 
@@ -2767,11 +2768,11 @@ static void ufshcd_prepare_utp_scsi_cmd_upiu(struct scsi_cmnd *cmd,
 		.transaction_code = UPIU_TRANSACTION_COMMAND,
 		.flags = upiu_flags,
 		.lun = lrbp->lun,
-		.task_tag = lrbp->task_tag,
+		.task_tag = tag,
 		.command_set_type = UPIU_COMMAND_SET_TYPE_SCSI,
 	};
 
-	WARN_ON_ONCE(ucd_req_ptr->header.task_tag != lrbp->task_tag);
+	WARN_ON_ONCE(ucd_req_ptr->header.task_tag != tag);
 
 	ucd_req_ptr->sc.exp_data_transfer_len = cpu_to_be32(cmd->sdb.length);
 
@@ -2791,6 +2792,8 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
 				struct ufshcd_lrb *lrbp, u8 upiu_flags)
 {
 	struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr;
+	struct scsi_cmnd *cmd = (struct scsi_cmnd *)lrbp - 1;
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	struct ufs_query *query = &hba->dev_cmd.query;
 	u16 len = be16_to_cpu(query->request.upiu_req.length);
 
@@ -2799,7 +2802,7 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
 		.transaction_code = UPIU_TRANSACTION_QUERY_REQ,
 		.flags = upiu_flags,
 		.lun = lrbp->lun,
-		.task_tag = lrbp->task_tag,
+		.task_tag = tag,
 		.query_function = query->request.query_func,
 		/* Data segment length only need for WRITE_DESC */
 		.data_segment_length =
@@ -2823,12 +2826,14 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
 static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp)
 {
 	struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr;
+	struct scsi_cmnd *cmd = (struct scsi_cmnd *)lrbp - 1;
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 
 	memset(ucd_req_ptr, 0, sizeof(struct utp_upiu_req));
 
 	ucd_req_ptr->header = (struct utp_upiu_header){
 		.transaction_code = UPIU_TRANSACTION_NOP_OUT,
-		.task_tag = lrbp->task_tag,
+		.task_tag = tag,
 	};
 
 	memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));
@@ -2864,7 +2869,7 @@ static int ufshcd_compose_devman_upiu(struct ufs_hba *hba,
  * ufshcd_comp_scsi_upiu - UFS Protocol Information Unit(UPIU)
  *			   for SCSI Purposes
  * @hba: per adapter instance
- * @lrbp: pointer to local reference block
+ * @cmd: SCSI command
  */
 static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
@@ -2912,7 +2917,6 @@ static void __ufshcd_setup_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 
 	memset(lrbp->ucd_req_ptr, 0, sizeof(*lrbp->ucd_req_ptr));
 
-	lrbp->task_tag = tag;
 	lrbp->lun = lun;
 	ufshcd_prepare_lrbp_crypto(cmd ? scsi_cmd_to_rq(cmd) : NULL, lrbp);
 }
@@ -3203,6 +3207,8 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
 		struct ufshcd_lrb *lrbp, int max_timeout)
 {
+	struct scsi_cmnd *cmd = (struct scsi_cmnd *)lrbp - 1;
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	unsigned long time_left = msecs_to_jiffies(max_timeout);
 	unsigned long flags;
 	bool pending;
@@ -3219,18 +3225,18 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
 	} else {
 		err = -ETIMEDOUT;
 		dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
-			__func__, lrbp->task_tag);
+			__func__, tag);
 
 		/* MCQ mode */
 		if (hba->mcq_enabled) {
 			/* successfully cleared the command, retry if needed */
-			if (ufshcd_clear_cmd(hba, lrbp->task_tag) == 0)
+			if (ufshcd_clear_cmd(hba, tag) == 0)
 				err = -EAGAIN;
 			return err;
 		}
 
 		/* SDB mode */
-		if (ufshcd_clear_cmd(hba, lrbp->task_tag) == 0) {
+		if (ufshcd_clear_cmd(hba, tag) == 0) {
 			/* successfully cleared the command, retry if needed */
 			err = -EAGAIN;
 			/*
@@ -3239,11 +3245,9 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
 			 * variable.
 			 */
 			spin_lock_irqsave(&hba->outstanding_lock, flags);
-			pending = test_bit(lrbp->task_tag,
-					   &hba->outstanding_reqs);
+			pending = test_bit(tag, &hba->outstanding_reqs);
 			if (pending)
-				__clear_bit(lrbp->task_tag,
-					    &hba->outstanding_reqs);
+				__clear_bit(tag, &hba->outstanding_reqs);
 			spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 
 			if (!pending) {
@@ -3256,11 +3260,10 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
 			}
 		} else {
 			dev_err(hba->dev, "%s: failed to clear tag %d\n",
-				__func__, lrbp->task_tag);
+				__func__, tag);
 
 			spin_lock_irqsave(&hba->outstanding_lock, flags);
-			pending = test_bit(lrbp->task_tag,
-					   &hba->outstanding_reqs);
+			pending = test_bit(tag, &hba->outstanding_reqs);
 			spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 
 			if (!pending) {
@@ -5384,6 +5387,7 @@ static inline int ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 					     struct cq_entry *cqe)
 {
 	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
+	const int tag = scsi_cmd_to_rq(cmd)->tag;
 	int result = 0;
 	int scsi_status;
 	enum utp_ocs ocs;
@@ -5455,10 +5459,8 @@ static inline int ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 	case OCS_ABORTED:
 	case OCS_INVALID_COMMAND_STATUS:
 		result |= DID_REQUEUE << 16;
-		dev_warn(hba->dev,
-				"OCS %s from controller for tag %d\n",
-				(ocs == OCS_ABORTED ? "aborted" : "invalid"),
-				lrbp->task_tag);
+		dev_warn(hba->dev, "OCS %s from controller for tag %d\n",
+			 ocs == OCS_ABORTED ? "aborted" : "invalid", tag);
 		break;
 	case OCS_INVALID_CMD_TABLE_ATTR:
 	case OCS_INVALID_PRDT_ATTR:
@@ -5471,9 +5473,8 @@ static inline int ufshcd_transfer_rsp_status(struct ufs_hba *hba,
 	case OCS_GENERAL_CRYPTO_ERROR:
 	default:
 		result |= DID_ERROR << 16;
-		dev_err(hba->dev,
-				"OCS error from controller = %x for tag %d\n",
-				ocs, lrbp->task_tag);
+		dev_err(hba->dev, "OCS error from controller = %x for tag %d\n",
+			ocs, tag);
 		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		break;
@@ -7540,8 +7541,8 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 	u8 resp = 0xF;
 
 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
-		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-				UFS_QUERY_TASK, &resp);
+		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, tag, UFS_QUERY_TASK,
+					  &resp);
 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
 			/* cmd pending in the device */
 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
@@ -7574,8 +7575,7 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 	if (!poll_cnt)
 		return -EBUSY;
 
-	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-			UFS_ABORT_TASK, &resp);
+	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, tag, UFS_ABORT_TASK, &resp);
 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
 		if (!err) {
 			err = resp; /* service response error */
@@ -7685,7 +7685,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto release;
 	}
 
-	err = ufshcd_try_to_abort_task(hba, lrbp->task_tag);
+	err = ufshcd_try_to_abort_task(hba, tag);
 	if (err) {
 		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
 		ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 3e7ee781b841..0e61d89e83ce 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -188,7 +188,6 @@ struct ufshcd_lrb {
 	int scsi_status;
 
 	int command_type;
-	int task_tag;
 	u8 lun; /* UPIU LUN id field is only 8-bit wide */
 	bool intr_cmd;
 	ktime_t issue_time_stamp;

Re: [PATCH 00/24] Optimize the hot path in the UFS driver

[John Garry]

On 03/04/2025 22:17, Bart Van Assche wrote:
> Hi Martin,
> 
> This patch series increases IOPS by 1% and reduces CPU per I/O by 10% on my
> UFS 4.0 test setup. Please consider this patch series for the next merge
> window.

That cover letter is a bit thin for something which now implements SCSI 
reserved command handling (in addition to any UFS optimisation).

> 
> Thanks,
> 
> Bart.
> 
> Bart Van Assche (23):
>    scsi: core: Make scsi_cmd_to_rq() accept const arguments
>    scsi: core: Make scsi_cmd_priv() accept const arguments
>    scsi: core: Use scsi_cmd_priv() instead of open-coding it
>    scsi: core: Introduce scsi_host_update_can_queue()
>    scsi: ufs: core: Change the type of one ufshcd_add_cmd_upiu_trace()
>      argument
>    scsi: ufs: core: Only call ufshcd_add_command_trace() for SCSI
>      commands
>    scsi: ufs: core: Change the type of one ufshcd_add_command_trace()
>      argument
>    scsi: ufs: core: Change the type of one ufshcd_send_command() argument
>    scsi: ufs: core: Only call ufshcd_should_inform_monitor() for SCSI
>      commands
>    scsi: ufs: core: Change the monitor function argument types
>    scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()
>    scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()
>    scsi: ufs: core: Cache the DMA buffer sizes
>    scsi: ufs: core: Add an argument to ufshcd_mcq_decide_queue_depth()
>    scsi: ufs: core: Add an argument to ufshcd_alloc_mcq()
>    scsi: ufs: core: Call ufshcd_mcq_init() once
>    scsi: ufs: core: Allocate the SCSI host earlier
>    scsi: ufs: core: Call ufshcd_init_lrb() later
>    scsi: ufs: core: Use hba->reserved_slot
>    scsi: ufs: core: Allocate the reserved slot as a reserved request
>    scsi: ufs: core: Do not clear driver-private command data
>    scsi: ufs: core: Optimize the hot path
>    scsi: ufs: core: Remove the ufshcd_lrb task_tag member
> 
> Hannes Reinecke (1):
>    scsi: core: Implement reserved command handling
> 
>   drivers/scsi/hosts.c             |   3 +
>   drivers/scsi/scsi.c              |  26 ++
>   drivers/scsi/scsi_lib.c          |   6 +-
>   drivers/scsi/scsi_logging.c      |  10 +-
>   drivers/ufs/core/ufs-mcq.c       |  31 +-
>   drivers/ufs/core/ufshcd-crypto.h |  18 +-
>   drivers/ufs/core/ufshcd-priv.h   |  27 +-
>   drivers/ufs/core/ufshcd.c        | 660 +++++++++++++++++--------------
>   include/scsi/scsi_cmnd.h         |  17 +-
>   include/scsi/scsi_host.h         |  24 +-
>   include/ufs/ufshcd.h             |  11 +-
>   11 files changed, 487 insertions(+), 346 deletions(-)
> 
> 

Re: [PATCH 01/24] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[John Garry]

On 03/04/2025 22:17, Bart Van Assche wrote:
>   /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */
> -static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
> -{
> -	return blk_mq_rq_from_pdu(scmd);
> -}
> +#define scsi_cmd_to_rq(scmd)                                       \
> +	_Generic(scmd,                                             \
> +		const struct scsi_cmnd *: (const struct request *) \
> +			blk_mq_rq_from_pdu((void *)scmd),          \
> +		struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
>   
>   /*

Out of curiosity, how this that better than this:

static inline struct request *scsi_cmd_to_rq(const struct scsi_cmnd *scmd)
{
	return blk_mq_rq_from_pdu((void *)scmd);
}

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[John Garry]

On 03/04/2025 22:17, Bart Van Assche wrote:
> From: Hannes Reinecke<hare@suse.de>
> 
> Quite some drivers are using management commands internally. These commands
> typically use the same tag pool as regular SCSI commands. Tags for these
> management commands are set aside before allocating the block-mq tag bitmap
> for regular SCSI commands. The block layer already supports this via the
> reserved tag mechanism. Add a new field 'nr_reserved_cmds' to the SCSI host
> template to instruct the block layer to set aside a tag space for these
> management commands by using reserved tags. Exclude reserved commands from
> .can_queue because .can_queue is visible in sysfs.
> 
> Signed-off-by: Hannes Reinecke<hare@suse.de>
> [ bvanassche: modified patch description ]
> Cc: John Garry<john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche<bvanassche@acm.org>

How is this related to the rest of the series?

To allocate a reserved-tag request we need to use BLK_MQ_REQ_RESERVED, 
right? I don't see that used...

Re: [PATCH 01/24] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Bart Van Assche]

On 4/4/25 3:39 AM, John Garry wrote:
> On 03/04/2025 22:17, Bart Van Assche wrote:
>>   /* Variant of blk_mq_rq_from_pdu() that verifies the type of its 
>> argument. */
>> -static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
>> -{
>> -    return blk_mq_rq_from_pdu(scmd);
>> -}
>> +#define scsi_cmd_to_rq(scmd)                                       \
>> +    _Generic(scmd,                                             \
>> +        const struct scsi_cmnd *: (const struct request *) \
>> +            blk_mq_rq_from_pdu((void *)scmd),          \
>> +        struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
>>   /*
> 
> Out of curiosity, how this that better than this:
> 
> static inline struct request *scsi_cmd_to_rq(const struct scsi_cmnd *scmd)
> {
>      return blk_mq_rq_from_pdu((void *)scmd);
> }

Yikes. With the inline function variant constness can be removed
silently as follows:

     const struct scsi_cmnd *scmd;
     struct request *rq = scsi_cmd_to_rq(scmd);

With patch 01/24, the compiler will issue a warning for the above code.

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/4/25 3:49 AM, John Garry wrote:
> On 03/04/2025 22:17, Bart Van Assche wrote:
>> From: Hannes Reinecke<hare@suse.de>
>>
>> Quite some drivers are using management commands internally. These 
>> commands
>> typically use the same tag pool as regular SCSI commands. Tags for these
>> management commands are set aside before allocating the block-mq tag 
>> bitmap
>> for regular SCSI commands. The block layer already supports this via the
>> reserved tag mechanism. Add a new field 'nr_reserved_cmds' to the SCSI 
>> host
>> template to instruct the block layer to set aside a tag space for these
>> management commands by using reserved tags. Exclude reserved commands 
>> from
>> .can_queue because .can_queue is visible in sysfs.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> [ bvanassche: modified patch description ]
>> Cc: John Garry<john.g.garry@oracle.com>
>> Signed-off-by: Bart Van Assche<bvanassche@acm.org>
> 
> How is this related to the rest of the series?
> 
> To allocate a reserved-tag request we need to use BLK_MQ_REQ_RESERVED, 
> right? I don't see that used...

Hi John,

Calling blk_mq_alloc_request(..., BLK_MQ_REQ_RESERVED) is not the only 
way to allocated a reserved tag. Another possibility is to let the
driver manage reserved tags. The UFS driver only needs a single reserved
tag and already serializes use of that tag. See also the following
change in patch 21/24:

-	hba->reserved_slot = hba->nutrs - 1;
+	hba->reserved_slot = 0;

Use of hba->reserved_slot is serialized by calling ufshcd_dev_man_lock()
and ufshcd_dev_man_unlock(). More code is serialized by these calls than
only the region in which hba->reserved_slot is used so I don't think
that the UFS driver code would become shorter by using block layer
functions for allocating / freeing reserved tags.

Thanks,

Bart.

Re: [PATCH 00/24] Optimize the hot path in the UFS driver

[Bart Van Assche]

On 4/4/25 3:15 AM, John Garry wrote:
> On 03/04/2025 22:17, Bart Van Assche wrote:
>> Hi Martin,
>>
>> This patch series increases IOPS by 1% and reduces CPU per I/O by 10% 
>> on my
>> UFS 4.0 test setup. Please consider this patch series for the next merge
>> window.
> 
> That cover letter is a bit thin for something which now implements SCSI 
> reserved command handling (in addition to any UFS optimisation).
Hi John,

Here is a more detailed version:

This patch series optimizes the hot path of the UFS driver by making
struct scsi_cmnd and struct ufshcd_lrb adjacent. Making these two data
structures adjacent is realized as follows:

@@ -9040,6 +9046,7 @@ static const struct scsi_host_template 
ufshcd_driver_template = {
  	.name			= UFSHCD,
  	.proc_name		= UFSHCD,
  	.map_queues		= ufshcd_map_queues,
+	.cmd_size		= sizeof(struct ufshcd_lrb),
  	.init_cmd_priv		= ufshcd_init_cmd_priv,
  	.queuecommand		= ufshcd_queuecommand,
  	.mq_poll		= ufshcd_poll,

The following changes had to be made prior to making these two data
structures adjacent:
* Instead of making the reserved command slot (hba->reserved_slot)
   invisible to the SCSI core, let the SCSI core allocate memory for the
   reserved slot and tell the block layer to reserve one slot. This is
   why patch 04/24 adds minimal support in the SCSI core for reserved
   command handling.
* Remove all UFS data structure members that are no longer needed
   because struct scsi_cmnd and struct ufshcd_lrb are now adjacent
* Call ufshcd_init_lrb() from inside ufshcd_queuecommand() instead of
   calling this function before I/O starts. This is necessary because
   ufshcd_memory_alloc() allocates fewer instances than the block layer
   allocates requests. See also the following code in the block layer
   core:

	if (blk_mq_init_request(set, hctx->fq->flush_rq, hctx_idx,
				hctx->numa_node))

   Although the UFS driver could be modified such that ufshcd_init_lrb()
   is called from ufshcd_init_cmd_priv(), realizing this would require
   moving the memory allocations that happen from inside
   ufshcd_memory_alloc() into ufshcd_init_cmd_priv(). That would make
   this patch series even larger.
* ufshcd_add_scsi_host() happens now before any device management
   commands are submitted. This change is necessary because this patch
   makes device management command allocation happen when the SCSI host
   is allocated.
* Start with allocating 32 command slots and increase this number later
   after it is clear whether or not the UFS device supports more than 32
   command slots. Introduce scsi_host_update_can_queue() to support this
   approach.

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[John Garry]

On 04/04/2025 17:34, Bart Van Assche wrote:
>>> Quite some drivers are using management commands internally. These 
>>> commands
>>> typically use the same tag pool as regular SCSI commands. Tags for these
>>> management commands are set aside before allocating the block-mq tag 
>>> bitmap
>>> for regular SCSI commands. The block layer already supports this via the
>>> reserved tag mechanism. Add a new field 'nr_reserved_cmds' to the 
>>> SCSI host
>>> template to instruct the block layer to set aside a tag space for these
>>> management commands by using reserved tags. Exclude reserved commands 
>>> from
>>> .can_queue because .can_queue is visible in sysfs.
>>>
>>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>>> [ bvanassche: modified patch description ]
>>> Cc: John Garry<john.g.garry@oracle.com>
>>> Signed-off-by: Bart Van Assche<bvanassche@acm.org>
>>
>> How is this related to the rest of the series?
>>
>> To allocate a reserved-tag request we need to use BLK_MQ_REQ_RESERVED, 
>> right? I don't see that used...
> 
> Hi John,
> 
> Calling blk_mq_alloc_request(..., BLK_MQ_REQ_RESERVED) is not the only 
> way to allocated a reserved tag. Another possibility is to let the
> driver manage reserved tags. The UFS driver only needs a single reserved
> tag and already serializes use of that tag. See also the following
> change in patch 21/24:
> 
> -    hba->reserved_slot = hba->nutrs - 1;
> +    hba->reserved_slot = 0;
> 
> Use of hba->reserved_slot is serialized by calling ufshcd_dev_man_lock()
> and ufshcd_dev_man_unlock(). More code is serialized by these calls than
> only the region in which hba->reserved_slot is used so I don't think
> that the UFS driver code would become shorter by using block layer
> functions for allocating / freeing reserved tags.

Now I see this in 23/24:

+/*
+ * Convert a block layer tag into a SCSI command pointer. This function is
+ * called once per I/O completion path and is also called from error paths.
+ */
+static inline struct scsi_cmnd *ufshcd_tag_to_cmd(struct ufs_hba *hba, 
u32 tag)
+{
+	struct blk_mq_tags *tags = hba->host->tag_set.tags[0];
+	struct request *rq;
+
+	/*
+	 * Use .static_rqs[] for reserved commands because blk_mq_get_tag()
+	 * is not called for reserved commands by the UFS driver.
+	 */
+	rq = tag < UFSHCD_NUM_RESERVED ? tags->static_rqs[tag] :
+					 blk_mq_tag_to_rq(tags, tag);
+
+	if (WARN_ON_ONCE(!rq))
+		return NULL;
+
+	return blk_mq_rq_to_pdu(rq);
+}
+

Do you really think that it is ok that anything outside the block layer 
should be referencing tags->static_rqs[] directly?

Even using blk_mq_alloc_request() would seem better than that.

RE: [PATCH 00/24] Optimize the hot path in the UFS driver

[Avri Altman]

> On 4/4/25 3:15 AM, John Garry wrote:
> > On 03/04/2025 22:17, Bart Van Assche wrote:
> >> Hi Martin,
> >>
> >> This patch series increases IOPS by 1% and reduces CPU per I/O by 10%
> >> on my UFS 4.0 test setup. Please consider this patch series for the
> >> next merge window.
> >
> > That cover letter is a bit thin for something which now implements
> > SCSI reserved command handling (in addition to any UFS optimisation).
> Hi John,
> 
> Here is a more detailed version:
> 
> This patch series optimizes the hot path of the UFS driver by making struct
> scsi_cmnd and struct ufshcd_lrb adjacent. Making these two data structures
> adjacent is realized as follows:
> 
> @@ -9040,6 +9046,7 @@ static const struct scsi_host_template
> ufshcd_driver_template = {
>   	.name			= UFSHCD,
>   	.proc_name		= UFSHCD,
>   	.map_queues		= ufshcd_map_queues,
> +	.cmd_size		= sizeof(struct ufshcd_lrb),
>   	.init_cmd_priv		= ufshcd_init_cmd_priv,
>   	.queuecommand		= ufshcd_queuecommand,
>   	.mq_poll		= ufshcd_poll,
> 
> The following changes had to be made prior to making these two data
> structures adjacent:
> * Instead of making the reserved command slot (hba->reserved_slot)
>    invisible to the SCSI core, let the SCSI core allocate memory for the
>    reserved slot and tell the block layer to reserve one slot. This is
>    why patch 04/24 adds minimal support in the SCSI core for reserved
>    command handling.
> * Remove all UFS data structure members that are no longer needed
>    because struct scsi_cmnd and struct ufshcd_lrb are now adjacent
> * Call ufshcd_init_lrb() from inside ufshcd_queuecommand() instead of
>    calling this function before I/O starts. This is necessary because
>    ufshcd_memory_alloc() allocates fewer instances than the block layer
>    allocates requests. See also the following code in the block layer
>    core:
> 
> 	if (blk_mq_init_request(set, hctx->fq->flush_rq, hctx_idx,
> 				hctx->numa_node))
> 
>    Although the UFS driver could be modified such that ufshcd_init_lrb()
>    is called from ufshcd_init_cmd_priv(), realizing this would require
>    moving the memory allocations that happen from inside
>    ufshcd_memory_alloc() into ufshcd_init_cmd_priv(). That would make
>    this patch series even larger.
> * ufshcd_add_scsi_host() happens now before any device management
>    commands are submitted. This change is necessary because this patch
>    makes device management command allocation happen when the SCSI host
>    is allocated.
> * Start with allocating 32 command slots and increase this number later
>    after it is clear whether or not the UFS device supports more than 32
>    command slots. Introduce scsi_host_update_can_queue() to support this
>    approach.
Maybe add one extra sentence to your commit log, now that ufshcd_init_lrb() is called on each command:
The benefits of reduced indirection and better cache efficiency outweigh the small overhead of per-command lrb initialization in most high-throughput workloads.

Thanks,
Avri

> 
> Bart.

RE: [PATCH 06/24] scsi: ufs: core: Change the type of one ufshcd_add_cmd_upiu_trace() argument

[Avri Altman]

> Subject: [PATCH 06/24] scsi: ufs: core: Change the type of one
> ufshcd_add_cmd_upiu_trace() argument
> 
> Change the 'tag' argument into an LRB pointer. This patch prepares for the
> removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>

> ---
>  drivers/ufs/core/ufshcd.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> c4448b94092f..3b470f564313 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -355,10 +355,11 @@ static void ufshcd_configure_wb(struct ufs_hba *hba)
>  		ufshcd_wb_toggle_buf_flush(hba, true);  }
> 
> -static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
> +static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba,
> +				      struct ufshcd_lrb *lrb,
>  				      enum ufs_trace_str_t str_t)
>  {
> -	struct utp_upiu_req *rq = hba->lrb[tag].ucd_req_ptr;
> +	struct utp_upiu_req *rq = lrb->ucd_req_ptr;
>  	struct utp_upiu_header *header;
> 
>  	if (!trace_ufshcd_upiu_enabled())
> @@ -367,7 +368,7 @@ static void ufshcd_add_cmd_upiu_trace(struct ufs_hba
> *hba, unsigned int tag,
>  	if (str_t == UFS_CMD_SEND)
>  		header = &rq->header;
>  	else
> -		header = &hba->lrb[tag].ucd_rsp_ptr->header;
> +		header = &lrb->ucd_rsp_ptr->header;
> 
>  	trace_ufshcd_upiu(hba, str_t, header, &rq->sc.cdb,
>  			  UFS_TSF_CDB);
> @@ -441,7 +442,7 @@ static void ufshcd_add_command_trace(struct ufs_hba
> *hba, unsigned int tag,
>  		return;
> 
>  	/* trace UPIU also */
> -	ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
> +	ufshcd_add_cmd_upiu_trace(hba, lrbp, str_t);
>  	if (!trace_ufshcd_command_enabled())
>  		return;
> 

RE: [PATCH 07/24] scsi: ufs: core: Only call ufshcd_add_command_trace() for SCSI commands

[Avri Altman]

> Instead of checking inside ufshcd_add_command_trace() whether 'cmd' points at
> a SCSI command, let the caller perform that check. This patch prepares for
> removing the lrbp->cmd pointer.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>

> ---
>  drivers/ufs/core/ufshcd.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 3b470f564313..94cf864ac62b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -438,9 +438,6 @@ static void ufshcd_add_command_trace(struct ufs_hba
> *hba, unsigned int tag,
>  	struct request *rq = scsi_cmd_to_rq(cmd);
>  	int transfer_len = -1;
> 
> -	if (!cmd)
> -		return;
> -
>  	/* trace UPIU also */
>  	ufshcd_add_cmd_upiu_trace(hba, lrbp, str_t);
>  	if (!trace_ufshcd_command_enabled())
> @@ -2309,9 +2306,10 @@ void ufshcd_send_command(struct ufs_hba *hba,
> unsigned int task_tag,
>  	lrbp->issue_time_stamp_local_clock = local_clock();
>  	lrbp->compl_time_stamp = ktime_set(0, 0);
>  	lrbp->compl_time_stamp_local_clock = 0;
> -	ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
> -	if (lrbp->cmd)
> +	if (lrbp->cmd) {
> +		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
>  		ufshcd_clk_scaling_start_busy(hba);
> +	}
>  	if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
>  		ufshcd_start_monitor(hba, lrbp);
> 

RE: [PATCH 08/24] scsi: ufs: core: Change the type of one ufshcd_add_command_trace() argument

[Avri Altman]

 
> Change the 'tag' argument into a SCSI command pointer. This patch prepares for
> the removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Why cmd and not lrbp? Seems like a step backwards.

Anyway,
Reviewed-by: Avri Altman <avri.altman@sandisk.com>

> ---
>  drivers/ufs/core/ufshcd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 94cf864ac62b..f87526443d8d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -425,7 +425,7 @@ static void ufshcd_add_uic_command_trace(struct
> ufs_hba *hba,
>  				 ufshcd_readl(hba,
> REG_UIC_COMMAND_ARG_3));  }
> 
> -static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
> +static void ufshcd_add_command_trace(struct ufs_hba *hba, struct
> +scsi_cmnd *cmd,
>  				     enum ufs_trace_str_t str_t)
>  {
>  	u64 lba = 0;
> @@ -433,9 +433,9 @@ static void ufshcd_add_command_trace(struct ufs_hba
> *hba, unsigned int tag,
>  	u32 doorbell = 0;
>  	u32 intr;
>  	int hwq_id = -1;
> -	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> -	struct scsi_cmnd *cmd = lrbp->cmd;
>  	struct request *rq = scsi_cmd_to_rq(cmd);
> +	unsigned int tag = rq->tag;
> +	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>  	int transfer_len = -1;
> 
>  	/* trace UPIU also */
> @@ -453,7 +453,7 @@ static void ufshcd_add_command_trace(struct ufs_hba
> *hba, unsigned int tag,
>  		       be32_to_cpu(lrbp->ucd_req_ptr-
> >sc.exp_data_transfer_len);
>  		lba = scsi_get_lba(cmd);
>  		if (opcode == WRITE_10)
> -			group_id = lrbp->cmd->cmnd[6];
> +			group_id = cmd->cmnd[6];
>  	} else if (opcode == UNMAP) {
>  		/*
>  		 * The number of Bytes to be unmapped beginning with the lba.
> @@ -2307,7 +2307,7 @@ void ufshcd_send_command(struct ufs_hba *hba,
> unsigned int task_tag,
>  	lrbp->compl_time_stamp = ktime_set(0, 0);
>  	lrbp->compl_time_stamp_local_clock = 0;
>  	if (lrbp->cmd) {
> -		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_SEND);
> +		ufshcd_add_command_trace(hba, lrbp->cmd, UFS_CMD_SEND);
>  		ufshcd_clk_scaling_start_busy(hba);
>  	}
>  	if (unlikely(ufshcd_should_inform_monitor(hba, lrbp))) @@ -5564,7
> +5564,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>  	if (cmd) {
>  		if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
>  			ufshcd_update_monitor(hba, lrbp);
> -		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);
> +		ufshcd_add_command_trace(hba, cmd, UFS_CMD_COMP);
>  		cmd->result = ufshcd_transfer_rsp_status(hba, lrbp, cqe);
>  		ufshcd_release_scsi_cmd(hba, lrbp);
>  		/* Do not touch lrbp after scsi done */

RE: [PATCH 09/24] scsi: ufs: core: Change the type of one ufshcd_send_command() argument

[Avri Altman]

> Change the 'task_tag' argument into an LRB pointer. This patch prepares for the
> removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@sandisk.com>

> ---
>  drivers/ufs/core/ufshcd.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> f87526443d8d..883551274330 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2292,14 +2292,13 @@ static void ufshcd_update_monitor(struct ufs_hba
> *hba, const struct ufshcd_lrb *
>  /**
>   * ufshcd_send_command - Send SCSI or device management commands
>   * @hba: per adapter instance
> - * @task_tag: Task tag of the command
> + * @lrbp: Local reference block of SCSI command
>   * @hwq: pointer to hardware queue instance
>   */
> -static inline
> -void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag,
> -			 struct ufs_hw_queue *hwq)
> +static inline void ufshcd_send_command(struct ufs_hba *hba,
> +				       struct ufshcd_lrb *lrbp,
> +				       struct ufs_hw_queue *hwq)
>  {
> -	struct ufshcd_lrb *lrbp = &hba->lrb[task_tag];
>  	unsigned long flags;
> 
>  	lrbp->issue_time_stamp = ktime_get();
> @@ -3029,7 +3028,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *cmd)
>  	if (hba->mcq_enabled)
>  		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
> 
> -	ufshcd_send_command(hba, tag, hwq);
> +	ufshcd_send_command(hba, lrbp, hwq);
> 
>  out:
>  	if (ufs_trigger_eh(hba)) {
> @@ -3263,7 +3262,7 @@ static int ufshcd_issue_dev_cmd(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp,
>  	int err;
> 
>  	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp-
> >ucd_req_ptr);
> -	ufshcd_send_command(hba, tag, hba->dev_cmd_queue);
> +	ufshcd_send_command(hba, lrbp, hba->dev_cmd_queue);
>  	err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
> 
>  	ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR :
> UFS_QUERY_COMP,

RE: [PATCH 17/24] scsi: ufs: core: Call ufshcd_mcq_init() once

[Avri Altman]

> Make sure that ufshcd_mcq_init() is called once even if ufshcd_alloc_mcq() is
> called twice.
Maybe elaborate the commit log, or even the cover letter,
explaining how patches 14..20 fits in the bigger picture of removing the lrb array.
Because it seems like you are going through a lot of trouble,
but essentially the queue allocation flow stays a 2-phase process as before.

Thanks,
Avri

> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> acbf173a3732..6dcac4143f4f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8733,9 +8733,16 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba, u32
> ufs_dev_qd)
>  		return ret;
> 
>  	hba->nutrs = ret;
> -	ret = ufshcd_mcq_init(hba);
> -	if (ret)
> -		goto err;
> +	if (hba->host->nr_hw_queues == 0) {
> +		/*
> +		 * ufshcd_mcq_init() is independent of hba->nutrs. Hence, only
> +		 * call ufshcd_mcq_init() the first time ufshcd_alloc_mcq() is
> +		 * called.
> +		 */
> +		ret = ufshcd_mcq_init(hba);
> +		if (ret)
> +			goto err;
> +	}
> 
>  	/*
>  	 * Previously allocated memory for nutrs may not be enough in MCQ
> mode.

Re: [PATCH 08/24] scsi: ufs: core: Change the type of one ufshcd_add_command_trace() argument

[Bart Van Assche]

On 4/8/25 11:32 PM, Avri Altman wrote:
>   
>> Change the 'tag' argument into a SCSI command pointer. This patch prepares for
>> the removal of the hba->lrb[] array.
>>
> Why cmd and not lrbp? Seems like a step backwards.

Hi Avri,

It is not clear to me why this is considered a step backwards? A later
patch makes struct scsi_cmnd and struct ufshcd_lrb adjacent. This means
that converting one pointer into the other involves adding or
subtracting an integer offset. Using struct scsi_cmnd everywhere to
refer to both struct scsi_cmnd and struct ufshcd_lrb has the advantage
that only SCSI command pointers have to be converted into LRB pointers
and that the opposite conversion is never necessary.

Thanks,

Bart.

Re: [PATCH 17/24] scsi: ufs: core: Call ufshcd_mcq_init() once

[Bart Van Assche]

On 4/8/25 11:52 PM, Avri Altman wrote:
>> Make sure that ufshcd_mcq_init() is called once even if ufshcd_alloc_mcq() is
>> called twice.
> Maybe elaborate the commit log, or even the cover letter,
> explaining how patches 14..20 fits in the bigger picture of removing the lrb array.
> Because it seems like you are going through a lot of trouble,
> but essentially the queue allocation flow stays a 2-phase process as before.

Hi Avri,

Making struct scsi_cmnd and struct ufshcd_lrb adjacent implies
allocating the SCSI host before any device management commands are sent
to the UFS device and before the UFS device queue depth has been
queried. Hence, the queue depth must be modified after the UFS device
queue depth has been queried. Modifiying the queue depth involves
calling ufshcd_alloc_mcq() a second time. This patch prepares for
calling ufshcd_alloc_mcq() twice.

Thanks,

Bart.

Re: [PATCH 06/24] scsi: ufs: core: Change the type of one ufshcd_add_cmd_upiu_trace() argument

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Change the 'tag' argument into an LRB pointer. This patch prepares
> for the
> removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 07/24] scsi: ufs: core: Only call ufshcd_add_command_trace() for SCSI commands

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Instead of checking inside ufshcd_add_command_trace() whether 'cmd'
> points
> at a SCSI command, let the caller perform that check. This patch
> prepares
> for removing the lrbp->cmd pointer.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 08/24] scsi: ufs: core: Change the type of one ufshcd_add_command_trace() argument

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Change the 'tag' argument into a SCSI command pointer. This patch
> prepares
> for the removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 09/24] scsi: ufs: core: Change the type of one ufshcd_send_command() argument

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Change the 'task_tag' argument into an LRB pointer. This patch
> prepares
> for the removal of the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/4/25 10:34 AM, John Garry wrote:
> Now I see this in 23/24:
> 
> +/*
> + * Convert a block layer tag into a SCSI command pointer. This function is
> + * called once per I/O completion path and is also called from error 
> paths.
> + */
> +static inline struct scsi_cmnd *ufshcd_tag_to_cmd(struct ufs_hba *hba, 
> u32 tag)
> +{
> +    struct blk_mq_tags *tags = hba->host->tag_set.tags[0];
> +    struct request *rq;
> +
> +    /*
> +     * Use .static_rqs[] for reserved commands because blk_mq_get_tag()
> +     * is not called for reserved commands by the UFS driver.
> +     */
> +    rq = tag < UFSHCD_NUM_RESERVED ? tags->static_rqs[tag] :
> +                     blk_mq_tag_to_rq(tags, tag);
> +
> +    if (WARN_ON_ONCE(!rq))
> +        return NULL;
> +
> +    return blk_mq_rq_to_pdu(rq);
> +}
> +
> 
> Do you really think that it is ok that anything outside the block layer 
> should be referencing tags->static_rqs[] directly?
> 
> Even using blk_mq_alloc_request() would seem better than that.

Hi John,

There is a challenge: a request queue must exist before
blk_mq_alloc_request() is called because the first argument of that
function is a request queue pointer. Creating a request queue by
calling __scsi_add_device() is not appropriate because
__scsi_add_device() submits SCSI commands to a SCSI device and
submitting SCSI commands to a UFS device must only happen after the
initial device management commands have been sent.

The only solution I see is to call blk_mq_init_queue() directly from the
UFS driver and to pass that request queue to blk_mq_alloc_request() when
allocating device management commands.

Bart.

Re: [PATCH 10/24] scsi: ufs: core: Only call ufshcd_should_inform_monitor() for SCSI commands

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> ufshcd_should_inform_monitor() only returns 'true' for SCSI commands.
> Instead of checking inside ufshcd_should_inform_monitor() whether its
> second argument represents a SCSI command, only call this function
> for
> SCSI commands. This patch prepares for removing the lrbp->cmd member.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> 

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 11/24] scsi: ufs: core: Change the monitor function argument types

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> 
> @@ -5562,8 +5565,8 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba,
> int task_tag,
>         lrbp->compl_time_stamp_local_clock = local_clock();
>         cmd = lrbp->cmd;
>         if (cmd) {
> -               if (unlikely(ufshcd_should_inform_monitor(hba,
> lrbp)))
> -                       ufshcd_update_monitor(hba, lrbp);
> +               if (unlikely(ufshcd_should_inform_monitor(hba, lrbp-
> >cmd)))
> +                       ufshcd_update_monitor(hba, lrbp->cmd);

Hi Bart,

Here could use "cmd" instead "lrbp->cmd"?

Thanks.
Peter

Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Peter Wang (王信友)]

On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Replace a tag loop with blk_mq_tagset_busy_iter(). This patch
> prepares
> for removing the hba->lrb[] array.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 74 ++++++++++++++++++++++---------------
> --
>  1 file changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 4b5734bbb12b..a5faf5af462e 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5651,6 +5651,44 @@ static int ufshcd_poll(struct Scsi_Host
> *shost, unsigned int queue_num)
>         return completed_reqs != 0;
>  }
> 
> +static bool ufshcd_mcq_force_compl_one(struct request *rq, void
> *priv)
> +{
> +       struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
> +       struct scsi_device *sdev = rq->q->queuedata;
> +       struct Scsi_Host *shost = sdev->host;
> +       struct ufs_hba *hba = shost_priv(shost);
> +       struct ufshcd_lrb *lrbp = &hba->lrb[rq->tag];
> +       struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
> +       unsigned long flags;
> +
> +       ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
> +       /*
> +        * For those cmds of which the cqes are not present
> +        * in the cq, complete them explicitly.
> +        */
> +       spin_lock_irqsave(&hwq->cq_lock, flags);
> +       if (cmd && !test_bit(SCMD_STATE_COMPLETE, &cmd->state)) {
> +               set_host_byte(cmd, DID_REQUEUE);
> +               ufshcd_release_scsi_cmd(hba, lrbp);
> +               scsi_done(cmd);
> +       }
> +       spin_unlock_irqrestore(&hwq->cq_lock, flags);
> +
> +       return true;
> +}
> +
> +static bool ufshcd_mcq_compl_one(struct request *rq, void *priv)
> +{
> +       struct scsi_device *sdev = rq->q->queuedata;
> +       struct Scsi_Host *shost = sdev->host;
> +       struct ufs_hba *hba = shost_priv(shost);
> +       struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
> +
> +       ufshcd_mcq_poll_cqe_lock(hba, hwq);
> +
> +       return true;
> +}
> +
>  /**
>   * ufshcd_mcq_compl_pending_transfer - MCQ mode function. It is
>   * invoked from the error handler context or
> ufshcd_host_reset_and_restore()
> @@ -5665,38 +5703,10 @@ static int ufshcd_poll(struct Scsi_Host
> *shost, unsigned int queue_num)
>  static void ufshcd_mcq_compl_pending_transfer(struct ufs_hba *hba,
>                                               bool force_compl)
>  {
> -       struct ufs_hw_queue *hwq;
> -       struct ufshcd_lrb *lrbp;
> -       struct scsi_cmnd *cmd;
> -       unsigned long flags;
> -       int tag;
> -
> -       for (tag = 0; tag < hba->nutrs; tag++) {
> -               lrbp = &hba->lrb[tag];
> -               cmd = lrbp->cmd;
> -               if (!ufshcd_cmd_inflight(cmd) ||
> -                   test_bit(SCMD_STATE_COMPLETE, &cmd->state))
> -                       continue;
> 

Hi Bart,

Removing this check might cause racing issues? 
Leading to the possibility that the hwq in the subsequent function
could be null?

Thanks.
Peter



> -
> -               hwq = ufshcd_mcq_req_to_hwq(hba,
> scsi_cmd_to_rq(cmd));
> -
> -               if (force_compl) {
> -                       ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
> -                       /*
> -                        * For those cmds of which the cqes are not
> present
> -                        * in the cq, complete them explicitly.
> -                        */
> -                       spin_lock_irqsave(&hwq->cq_lock, flags);
> -                       if (cmd && !test_bit(SCMD_STATE_COMPLETE,
> &cmd->state)) {
> -                               set_host_byte(cmd, DID_REQUEUE);
> -                               ufshcd_release_scsi_cmd(hba, lrbp);
> -                               scsi_done(cmd);
> -                       }
> -                       spin_unlock_irqrestore(&hwq->cq_lock, flags);
> -               } else {
> -                       ufshcd_mcq_poll_cqe_lock(hba, hwq);
> -               }
> -       }
> +       blk_mq_tagset_busy_iter(&hba->host->tag_set,
> +                               force_compl ?
> ufshcd_mcq_force_compl_one :
> +                                             ufshcd_mcq_compl_one,
> +                               NULL);
>  }
> 
>  /**

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/4/25 10:34 AM, John Garry wrote:
> Now I see this in 23/24:
> 
> +/*
> + * Convert a block layer tag into a SCSI command pointer. This function is
> + * called once per I/O completion path and is also called from error 
> paths.
> + */
> +static inline struct scsi_cmnd *ufshcd_tag_to_cmd(struct ufs_hba *hba, 
> u32 tag)
> +{
> +    struct blk_mq_tags *tags = hba->host->tag_set.tags[0];
> +    struct request *rq;
> +
> +    /*
> +     * Use .static_rqs[] for reserved commands because blk_mq_get_tag()
> +     * is not called for reserved commands by the UFS driver.
> +     */
> +    rq = tag < UFSHCD_NUM_RESERVED ? tags->static_rqs[tag] :
> +                     blk_mq_tag_to_rq(tags, tag);
> +
> +    if (WARN_ON_ONCE(!rq))
> +        return NULL;
> +
> +    return blk_mq_rq_to_pdu(rq);
> +}
> +
> 
> Do you really think that it is ok that anything outside the block layer 
> should be referencing tags->static_rqs[] directly?
> 
> Even using blk_mq_alloc_request() would seem better than that.

Hi John,

Using blk_mq_tag_to_rq() to translate a tag of a reserved
request into a request pointer only works after the block layer has
set tags->rqs[tag_of_reserved_request] first. That pointer is set by
blk_mq_get_driver_tag(). blk_mq_get_driver_tag() is called by the
block layer code that submits a request to a block driver. Hence, to
ensure that blk_mq_get_driver_tag() is called for reserved requests
the reserved requests would have to pass through scsi_queue_rq().
For reserved requests sdev == NULL as explained in a previous email.
There are many statements in the SCSI command submission and completion
path in which it is assumed that sdev != NULL. I don't think that the
SCSI maintainer (Martin) would agree with adding "if (sdev)" statements
in many places in the SCSI core.

Letting UFS reserved requests being processed by another function than
scsi_queue_rq() doesn't seem feasible to me either. Although it is easy
to create an additional request queue for reserved requests, that
request queue shares its tag set with the SCSI host and hence also the
request submission function. From scsi_host.h:

struct Scsi_Host {
	struct blk_mq_tag_set	tag_set;
	[ ... ]
};

 From blk-mq.h:

struct blk_mq_tag_set {
	const struct blk_mq_ops	*ops;
	[ ... ]
};

Unless someone comes up with an elegant proposal, I will keep the
approach where ufshcd_tag_to_cmd() handles reserved tags and regular
tags differently.

It should be possible to do this without referencing tags->static_rqs[]
directly from the UFS driver.

Bart.

Re: [PATCH 11/24] scsi: ufs: core: Change the monitor function argument types

[Bart Van Assche]

On 4/15/25 12:37 AM, Peter Wang (王信友) wrote:
> On Thu, 2025-04-03 at 14:17 -0700, Bart Van Assche wrote:
>> 
>> 
>> @@ -5562,8 +5565,8 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba,
>> int task_tag,
>>         lrbp->compl_time_stamp_local_clock = local_clock();
>>         cmd = lrbp->cmd;
>>         if (cmd) {
>> -               if (unlikely(ufshcd_should_inform_monitor(hba,
>> lrbp)))
>> -                       ufshcd_update_monitor(hba, lrbp);
>> +               if (unlikely(ufshcd_should_inform_monitor(hba, lrbp-
>> >cmd)))
>> +                       ufshcd_update_monitor(hba, lrbp->cmd);
> 
> Here could use "cmd" instead "lrbp->cmd"?
Hi Peter,

Sure, I will include these changes in this patch.

Thanks,

Bart.

Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Bart Van Assche]

On 4/15/25 1:00 AM, Peter Wang (王信友) wrote:
>>   /**
>>    * ufshcd_mcq_compl_pending_transfer - MCQ mode function. It is
>>    * invoked from the error handler context or
>> ufshcd_host_reset_and_restore()
>> @@ -5665,38 +5703,10 @@ static int ufshcd_poll(struct Scsi_Host
>> *shost, unsigned int queue_num)
>>   static void ufshcd_mcq_compl_pending_transfer(struct ufs_hba *hba,
>>                                                bool force_compl)
>>   {
>> -       struct ufs_hw_queue *hwq;
>> -       struct ufshcd_lrb *lrbp;
>> -       struct scsi_cmnd *cmd;
>> -       unsigned long flags;
>> -       int tag;
>> -
>> -       for (tag = 0; tag < hba->nutrs; tag++) {
>> -               lrbp = &hba->lrb[tag];
>> -               cmd = lrbp->cmd;
>> -               if (!ufshcd_cmd_inflight(cmd) ||
>> -                   test_bit(SCMD_STATE_COMPLETE, &cmd->state))
>> -                       continue;
>>
> 
> Removing this check might cause racing issues?
> Leading to the possibility that the hwq in the subsequent function
> could be null?

Hi Peter,

The ufshcd_cmd_inflight() check has not been removed.
blk_mq_tagset_busy_iter() only calls the callback function that is 
passed as second argument for requests that have been started. The
definition of ufshcd_cmd_inflight() is as follows:

bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd)
{
	return cmd &&
	       blk_mq_rq_state(scsi_cmd_to_rq(cmd)) == MQ_RQ_IN_FLIGHT;
}

 From the blk_mq_tagset_busy_iter() implementation:

	if (!(iter_data->flags & BT_TAG_ITER_STARTED) ||
	    blk_mq_request_started(rq))
		ret = iter_data->fn(rq, iter_data->data);

 From blk-mq.h:

static inline int blk_mq_request_started(struct request *rq)
{
	return blk_mq_rq_state(rq) != MQ_RQ_IDLE;
}

In other words, if flag BT_TAG_ITER_STARTED has not been set,
blk_mq_tagset_busy_iter() only calls its callback function for requests
for which blk_mq_start_request() has been called and that have not yet
been freed.

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[John Garry]

On 15/04/2025 18:21, Bart Van Assche wrote:
> Using blk_mq_tag_to_rq() to translate a tag of a reserved
> request into a request pointer only works after the block layer has
> set tags->rqs[tag_of_reserved_request] first. That pointer is set by
> blk_mq_get_driver_tag(). blk_mq_get_driver_tag() is called by the
> block layer code that submits a request to a block driver. Hence, to
> ensure that blk_mq_get_driver_tag() is called for reserved requests
> the reserved requests would have to pass through scsi_queue_rq().

Ah, yes, we had the same discussion when trying to implement SCSI 
reserved commands previously.

> For reserved requests sdev == NULL as explained in a previous email.

which mail exactly? I could not see any specific mention in this thread.

> There are many statements in the SCSI command submission and completion
> path in which it is assumed that sdev != NULL. I don't think that the
> SCSI maintainer (Martin) would agree with adding "if (sdev)" statements
> in many places in the SCSI core.

Sure, so, IIRC, Hannes solved this by using a shost sdev in 
https://lore.kernel.org/linux-scsi/20211125151048.103910-2-hare@suse.de/

> 
> Letting UFS reserved requests being processed by another function than
> scsi_queue_rq() doesn't seem feasible to me either.

JFYI, I was working on another solution (different to Hannes') which 
allows reserved requests pass through scsi_queue_rq(), but I stopped 
that work when I changed employer.

> Although it is easy
> to create an additional request queue for reserved requests, that
> request queue shares its tag set with the SCSI host and hence also the
> request submission function. From scsi_host.h:
> 
> struct Scsi_Host {
>      struct blk_mq_tag_set    tag_set;
>      [ ... ]
> };
> 
>  From blk-mq.h:
> 
> struct blk_mq_tag_set {
>      const struct blk_mq_ops    *ops;
>      [ ... ]
> };
> 
> Unless someone comes up with an elegant proposal, I will keep the
> approach where ufshcd_tag_to_cmd() handles reserved tags and regular
> tags differently.
> 
> It should be possible to do this without referencing tags->static_rqs[]
> directly from the UFS driver.

I'm not sure how that will look, but my preference is to fully implement 
reserved tags support which can be used by all SCSI LLDs.

Thanks,
John

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Hannes Reinecke]

On 4/15/25 19:21, Bart Van Assche wrote:
> 
> On 4/4/25 10:34 AM, John Garry wrote:
>> Now I see this in 23/24:
>>
>> +/*
>> + * Convert a block layer tag into a SCSI command pointer. This 
>> function is
>> + * called once per I/O completion path and is also called from error 
>> paths.
>> + */
>> +static inline struct scsi_cmnd *ufshcd_tag_to_cmd(struct ufs_hba 
>> *hba, u32 tag)
>> +{
>> +    struct blk_mq_tags *tags = hba->host->tag_set.tags[0];
>> +    struct request *rq;
>> +
>> +    /*
>> +     * Use .static_rqs[] for reserved commands because blk_mq_get_tag()
>> +     * is not called for reserved commands by the UFS driver.
>> +     */
>> +    rq = tag < UFSHCD_NUM_RESERVED ? tags->static_rqs[tag] :
>> +                     blk_mq_tag_to_rq(tags, tag);
>> +
>> +    if (WARN_ON_ONCE(!rq))
>> +        return NULL;
>> +
>> +    return blk_mq_rq_to_pdu(rq);
>> +}
>> +
>>
>> Do you really think that it is ok that anything outside the block 
>> layer should be referencing tags->static_rqs[] directly?
>>
>> Even using blk_mq_alloc_request() would seem better than that.
> 
> Hi John,
> 
> Using blk_mq_tag_to_rq() to translate a tag of a reserved
> request into a request pointer only works after the block layer has
> set tags->rqs[tag_of_reserved_request] first. That pointer is set by
> blk_mq_get_driver_tag(). blk_mq_get_driver_tag() is called by the
> block layer code that submits a request to a block driver. Hence, to
> ensure that blk_mq_get_driver_tag() is called for reserved requests
> the reserved requests would have to pass through scsi_queue_rq().
> For reserved requests sdev == NULL as explained in a previous email.
> There are many statements in the SCSI command submission and completion
> path in which it is assumed that sdev != NULL. I don't think that the
> SCSI maintainer (Martin) would agree with adding "if (sdev)" statements
> in many places in the SCSI core.
> 
> Letting UFS reserved requests being processed by another function than
> scsi_queue_rq() doesn't seem feasible to me either. Although it is easy
> to create an additional request queue for reserved requests, that
> request queue shares its tag set with the SCSI host and hence also the
> request submission function. From scsi_host.h:
> 

As rightly noted, we need an sdev to use the reserved command handling
trick. There are two choices:
-> use the existing sdev for per-LUN TMFs. If the driver only uses
    ABORT TASK/ABORT TASK_SET/RESET LUN we already know which sdev to
    use, so that is easy.
-> allocate a 'fake' scsi device for the host. Not pretty, but gives
    us a nice combined interface to deal with reserved commands.

Bart, from my reading the UFS driver only requires a TMF for command
abort and device reset, in both cases we should have a scsi device
available. Can't you use that?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Hannes Reinecke]

On 4/16/25 09:16, John Garry wrote:
> On 15/04/2025 18:21, Bart Van Assche wrote:
>> Using blk_mq_tag_to_rq() to translate a tag of a reserved
>> request into a request pointer only works after the block layer has
>> set tags->rqs[tag_of_reserved_request] first. That pointer is set by
>> blk_mq_get_driver_tag(). blk_mq_get_driver_tag() is called by the
>> block layer code that submits a request to a block driver. Hence, to
>> ensure that blk_mq_get_driver_tag() is called for reserved requests
>> the reserved requests would have to pass through scsi_queue_rq().
> 
> Ah, yes, we had the same discussion when trying to implement SCSI 
> reserved commands previously.
> 
>> For reserved requests sdev == NULL as explained in a previous email.
> 
> which mail exactly? I could not see any specific mention in this thread.
> 
>> There are many statements in the SCSI command submission and completion
>> path in which it is assumed that sdev != NULL. I don't think that the
>> SCSI maintainer (Martin) would agree with adding "if (sdev)" statements
>> in many places in the SCSI core.
> 
> Sure, so, IIRC, Hannes solved this by using a shost sdev in https:// 
> lore.kernel.org/linux-scsi/20211125151048.103910-2-hare@suse.de/
> 
>>
>> Letting UFS reserved requests being processed by another function than
>> scsi_queue_rq() doesn't seem feasible to me either.
> 
> JFYI, I was working on another solution (different to Hannes') which 
> allows reserved requests pass through scsi_queue_rq(), but I stopped 
> that work when I changed employer.
> 
Oh, really?
Do you have the patchset still around?
I'd be very much interested in that...

>> Although it is easy
>> to create an additional request queue for reserved requests, that
>> request queue shares its tag set with the SCSI host and hence also the
>> request submission function. From scsi_host.h:
>>
>> struct Scsi_Host {
>>      struct blk_mq_tag_set    tag_set;
>>      [ ... ]
>> };
>>
>>  From blk-mq.h:
>>
>> struct blk_mq_tag_set {
>>      const struct blk_mq_ops    *ops;
>>      [ ... ]
>> };
>>
>> Unless someone comes up with an elegant proposal, I will keep the
>> approach where ufshcd_tag_to_cmd() handles reserved tags and regular
>> tags differently.
>>
>> It should be possible to do this without referencing tags->static_rqs[]
>> directly from the UFS driver.
> 
> I'm not sure how that will look, but my preference is to fully implement 
> reserved tags support which can be used by all SCSI LLDs.
> 
Sure, that was my goal, too.
But then I got stuck as some drivers (most notably aacraid) use internal
commands to detect the hardware and allocate SCSI devices, so with my
approach we need a 'fake' SCSI device for the host to handle that.
If you have other ideas I'd be very interested in thems.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[John Garry]

On 16/04/2025 08:39, Hannes Reinecke wrote:
>> JFYI, I was working on another solution (different to Hannes') which 
>> allows reserved requests pass through scsi_queue_rq(), but I stopped 
>> that work when I changed employer.
>>
> Oh, really?
> Do you have the patchset still around?
> I'd be very much interested in that...

Yeah, if you remember, I was working on that for libsas/libata, but 
started to get bogged down in how to handle reserved commands for 
libsas/libata:

https://lore.kernel.org/linux-scsi/1666693096-180008-1-git-send-email-john.garry@huawei.com/T/#m35ad9a27320077f55c2be5549f036628fe79dcf7

It would be a lot simpler to support for HBAs which plug directly into 
the SCSI midlayer - like aacraid.

> 
>>> Although it is easy
>>> to create an additional request queue for reserved requests, that
>>> request queue shares its tag set with the SCSI host and hence also the
>>> request submission function. From scsi_host.h:
>>>
>>> struct Scsi_Host {
>>>      struct blk_mq_tag_set    tag_set;
>>>      [ ... ]
>>> };
>>>
>>>  From blk-mq.h:
>>>
>>> struct blk_mq_tag_set {
>>>      const struct blk_mq_ops    *ops;
>>>      [ ... ]
>>> };
>>>
>>> Unless someone comes up with an elegant proposal, I will keep the
>>> approach where ufshcd_tag_to_cmd() handles reserved tags and regular
>>> tags differently.
>>>
>>> It should be possible to do this without referencing tags->static_rqs[]
>>> directly from the UFS driver.
>>
>> I'm not sure how that will look, but my preference is to fully 
>> implement reserved tags support which can be used by all SCSI LLDs.
>>
> Sure, that was my goal, too.
> But then I got stuck as some drivers (most notably aacraid) use internal
> commands to detect the hardware and allocate SCSI devices, so with my
> approach we need a 'fake' SCSI device for the host to handle that.
> If you have other ideas I'd be very interested in thems.

My solution still involved supporting a fake sdev also for times like 
you describe, e.g. shost needs to send reserved commands.

Thanks,
John

Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Peter Wang (王信友)]

On Tue, 2025-04-15 at 13:22 -0700, Bart Van Assche wrote:
> Hi Peter,
> 
> The ufshcd_cmd_inflight() check has not been removed.
> blk_mq_tagset_busy_iter() only calls the callback function that is
> passed as second argument for requests that have been started. The
> definition of ufshcd_cmd_inflight() is as follows:
> 
> bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd)
> {
>         return cmd &&
>                blk_mq_rq_state(scsi_cmd_to_rq(cmd)) ==
> MQ_RQ_IN_FLIGHT;
> }
> 
>  From the blk_mq_tagset_busy_iter() implementation:
> 
>         if (!(iter_data->flags & BT_TAG_ITER_STARTED) ||
>             blk_mq_request_started(rq))
>                 ret = iter_data->fn(rq, iter_data->data);
> 
>  From blk-mq.h:
> 
> static inline int blk_mq_request_started(struct request *rq)
> {
>         return blk_mq_rq_state(rq) != MQ_RQ_IDLE;
> }
> 
> In other words, if flag BT_TAG_ITER_STARTED has not been set,
> blk_mq_tagset_busy_iter() only calls its callback function for
> requests
> for which blk_mq_start_request() has been called and that have not
> yet
> been freed.
> 
> Bart.

Hi Bart,

If blk_mq_rq_state(rq) state is MQ_RQ_COMPLETE, the code 
before modification would skip this tag.
But the code after modification will execute the function(fn). 
This should cause issues, right?

Thanks.
Peter

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/16/25 12:16 AM, John Garry wrote:
> I'm not sure how that will look, but my preference is to fully implement 
> reserved tags support which can be used by all SCSI LLDs.

Hi John,

I'm working on an implementation of this approach but it will take until
next month until I will have the time to post a patch series that
implements this approach.

Bart.

Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Bart Van Assche]

On 4/17/25 5:39 AM, Peter Wang (王信友) wrote:
> If blk_mq_rq_state(rq) state is MQ_RQ_COMPLETE, the code
> before modification would skip this tag.
> But the code after modification will execute the function(fn).
> This should cause issues, right?

Hi Peter,

My understanding is that the function modified by patch 12/24,
ufshcd_mcq_compl_pending_transfer(), is only called by the UFS error
handler. The UFS error handler is only activated after all pending
requests have completed (MQ_RQ_IDLE) or timed out (MQ_RQ_STARTED).
No requests should have the state MQ_RQ_COMPLETE when the error handler
is activated.

Thanks,

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Hannes Reinecke]

On 4/17/25 23:25, Bart Van Assche wrote:
> On 4/16/25 12:16 AM, John Garry wrote:
>> I'm not sure how that will look, but my preference is to fully 
>> implement reserved tags support which can be used by all SCSI LLDs.
> 
> Hi John,
> 
> I'm working on an implementation of this approach but it will take until
> next month until I will have the time to post a patch series that
> implements this approach.
> 
If it helps I can revisit and rebase my original patchset.
It had been shelved as there have been issues with the 'fnic' driver,
but as I now have a test system I can restart work on that series.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Peter Wang (王信友)]

On Thu, 2025-04-17 at 14:33 -0700, Bart Van Assche wrote:
> Hi Peter,
> 
> My understanding is that the function modified by patch 12/24,
> ufshcd_mcq_compl_pending_transfer(), is only called by the UFS error
> handler. The UFS error handler is only activated after all pending
> requests have completed (MQ_RQ_IDLE) or timed out (MQ_RQ_STARTED).
> No requests should have the state MQ_RQ_COMPLETE when the error
> handler
> is activated.
> 
> Thanks,
> 
> Bart.

Hi Bart,

Yes, ufshcd_mcq_compl_pending_transfer() is only called form error
handler.
Consider a situation where a UIC error occurs and triggers the error
handler, 
followed by receiving a request complete interrupt. 
At this point, the error handler might check this request that hasn't
completed yet, 
and just as it is about to complete it, the tag has already been
completed by 
the ISR. This request status may in MQ_RQ_COMPLETE.

Thanks.
Peter

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/17/25 2:25 PM, Bart Van Assche wrote:
> On 4/16/25 12:16 AM, John Garry wrote:
>> I'm not sure how that will look, but my preference is to fully 
>> implement reserved tags support which can be used by all SCSI LLDs.
> 
> I'm working on an implementation of this approach but it will take until
> next month until I will have the time to post a patch series that
> implements this approach.

(replying to my own email from four months ago)

Hi John,

With this approach and with the UFSHCI 4.0 controller model I have on my
desk, I'm hitting a hardware bug in the controller. I see completions
where cqe->command_desc_base_addr is NULL although I triple checked that
this pointer is not NULL in any submission queue entry. Let's postpone
the conversion to allocating reserved requests via the block layer until
I have a setup on which I can test this conversion.

Thanks,

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 8/5/25 3:33 PM, Bart Van Assche wrote:
> On 4/17/25 2:25 PM, Bart Van Assche wrote:
>> On 4/16/25 12:16 AM, John Garry wrote:
>>> I'm not sure how that will look, but my preference is to fully 
>>> implement reserved tags support which can be used by all SCSI LLDs.
>>
>> I'm working on an implementation of this approach but it will take until
>> next month until I will have the time to post a patch series that
>> implements this approach.
> 
> (replying to my own email from four months ago)
> 
> Hi John,
> 
> With this approach and with the UFSHCI 4.0 controller model I have on my
> desk, I'm hitting a hardware bug in the controller. I see completions
> where cqe->command_desc_base_addr is NULL although I triple checked that
> this pointer is not NULL in any submission queue entry. Let's postpone
> the conversion to allocating reserved requests via the block layer until
> I have a setup on which I can test this conversion.

(replying to my own email)

I found an elegant workaround for the hardware bug that I ran into and
will repost the entire patch series after the merge window has closed.

Bart.

Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Bart Van Assche]

On 4/16/25 12:33 AM, Hannes Reinecke wrote:
> As rightly noted, we need an sdev to use the reserved command handling
> trick. There are two choices:
> -> use the existing sdev for per-LUN TMFs. If the driver only uses
>     ABORT TASK/ABORT TASK_SET/RESET LUN we already know which sdev to
>     use, so that is easy.
> -> allocate a 'fake' scsi device for the host. Not pretty, but gives
>     us a nice combined interface to deal with reserved commands.
> 
> Bart, from my reading the UFS driver only requires a TMF for command
> abort and device reset, in both cases we should have a scsi device
> available. Can't you use that?

(replying to an email from four months ago)

The UFS driver submits multiple reserved commands to the UFS device
before LUN scanning starts, e.g. to initialize the UFS device.
Submitting these reserved commands to the UFS device after the WLUN has
been scanned is not possible. So I prefer the pseudo SCSI device
approach. I'm referring here to all the code that is called by
ufshcd_init() after the SCSI host has been added
(ufshcd_add_scsi_host(hba)) and before LUN scanning happens
(async_schedule(ufshcd_async_scan, hba)).

Thanks,

Bart.

Re: [PATCH 01/24] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Hannes Reinecke]

On 4/3/25 23:17, Bart Van Assche wrote:
> Instead of requiring the caller to cast away constness, make
> scsi_cmd_to_rq() accept const arguments.
> 
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_logging.c | 10 +++++-----
>   include/scsi/scsi_cmnd.h    |  9 +++++----
>   2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
> index b02af340c2d3..5aaff629b999 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -28,7 +28,7 @@ static void scsi_log_release_buffer(char *bufptr)
>   
>   static inline const char *scmd_name(const struct scsi_cmnd *scmd)
>   {
> -	struct request *rq = scsi_cmd_to_rq((struct scsi_cmnd *)scmd);
> +	const struct request *rq = scsi_cmd_to_rq(scmd);
>   
>   	if (!rq->q || !rq->q->disk)
>   		return NULL;
> @@ -94,7 +94,7 @@ void scmd_printk(const char *level, const struct scsi_cmnd *scmd,
>   	if (!logbuf)
>   		return;
>   	off = sdev_format_header(logbuf, logbuf_len, scmd_name(scmd),
> -				 scsi_cmd_to_rq((struct scsi_cmnd *)scmd)->tag);
> +				 scsi_cmd_to_rq(scmd)->tag);
>   	if (off < logbuf_len) {
>   		va_start(args, fmt);
>   		off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
> @@ -374,8 +374,8 @@ EXPORT_SYMBOL(__scsi_print_sense);
>   void scsi_print_sense(const struct scsi_cmnd *cmd)
>   {
>   	scsi_log_print_sense(cmd->device, scmd_name(cmd),
> -			     scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag,
> -			     cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
> +			     scsi_cmd_to_rq(cmd)->tag, cmd->sense_buffer,
> +			     SCSI_SENSE_BUFFERSIZE);
>   }
>   EXPORT_SYMBOL(scsi_print_sense);
>   
> @@ -393,7 +393,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
>   		return;
>   
>   	off = sdev_format_header(logbuf, logbuf_len, scmd_name(cmd),
> -				 scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag);
> +				 scsi_cmd_to_rq(cmd)->tag);
>   
>   	if (off >= logbuf_len)
>   		goto out_printk;
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 8ecfb94049db..154fbb39ca0c 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -144,10 +144,11 @@ struct scsi_cmnd {
>   };
>   
>   /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */
> -static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
> -{
> -	return blk_mq_rq_from_pdu(scmd);
> -}
> +#define scsi_cmd_to_rq(scmd)                                       \
> +	_Generic(scmd,                                             \
> +		const struct scsi_cmnd *: (const struct request *) \
> +			blk_mq_rq_from_pdu((void *)scmd),          \
> +		struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
>   
>   /*
>    * Return the driver private allocation behind the command.

Is there a cover letter for this patchset?
My mailer bunched it up under the first patch, leading to the wrong 
impression that it's just about converting to 'const' arguments...
(And me nearly ignoring the patchset altogether).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 01/24] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Bart Van Assche]

On 8/17/25 11:03 PM, Hannes Reinecke wrote:
> Is there a cover letter for this patchset?
> My mailer bunched it up under the first patch, leading to the wrong 
> impression that it's just about converting to 'const' arguments...
> (And me nearly ignoring the patchset altogether).

Hi Hannes,

The cover letter is available here:

https://lore.kernel.org/linux-scsi/20250403211937.2225615-1-bvanassche@acm.org/

A more detailed description of the purpose of this patch series than
what is available in the cover letter is available here:

https://lore.kernel.org/linux-scsi/3affc917-6634-47fc-a6d2-5b57a2e34bef@acm.org/

Thanks,

Bart.