Skip to contentSkip to conversation

mpt2/mpt3sas lock reduction for scsi-mq

RFC PATCH Superseded Jens Axboe 3 participants 15 messages Apr 3, 2015 – Apr 7, 2015

Subsystems (3)
SCSI Block layer LSI Logic MPT fusion drivers (FC/SAS/SPI)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jens Axboe

author
#0 lore raw

RFC PATCH 1/1 mpt2/mpt3sas lock reduction for scsi-mq

Hi,

There's some "low" hanging fruit in the lsi mpt drivers for further lock reduction, when running in scsi-mq mode. For each IO, they have to lock/{get,put}/unlock a scsiio_tracker structure that is associated with the IO. In scsi-mq mode, we can ask for some payload for each command, so we don't have to store and manage this tracker structure separately.

In some testing on mpt2sas, this roughly cuts the locking time in half at just 100K IOPS. Compared to scsi-mq not being enabled, the locking time with the patchset is reduced from ~47% to just ~4%.

Signature · 1 line
Jens Axboe
author
#1 lore raw

PATCH 1/7 blk-mq: allow the callback to blk_mq_tag_busy_iter() to stop looping

Currently blk_mq_tag_busy_iter() loops all busy tags for a given hardware queue. But sometimes we are looking for a specific request, and when we find it, we don't have to keep looking over the rest of them. Change the busy_iter_fn callback to return a bool, where a true return will break out of the search.

Update current callers (blk-mq timeout and NVMe IO cancel).

Signed-off-by Jens Axboe <[email protected]>
---
 block/blk-mq-tag.c        |  6 ++++--
 block/blk-mq.c            | 10 ++++++----
 drivers/block/nvme-core.c |  7 ++++---
 include/linux/blk-mq.h    |  2 +-
 4 files changed, 15 insertions(+), 10 deletions(-)
block/blk-mq-tag.c +4 -2
index be3290cc0644..129a881b8ef1 100644
@@ -430,8 +430,10 @@ static void bt_for_each(struct blk_mq_hw_ctx *hctx,
 		     bit < bm->depth;
 		     bit = find_next_bit(&bm->word, bm->depth, bit + 1)) {
 		     	rq = blk_mq_tag_to_rq(hctx->tags, off + bit);
-			if (rq->q == hctx->queue)
-				fn(hctx, rq, data, reserved);
+			if (rq->q != hctx->queue)
+				continue;
+			if (fn(hctx, rq, data, reserved))
+				break;
 		}
 
 		off += (1 << bt->bits_per_word);
block/blk-mq.c +6 -4
index b7b8933ec241..1cd34d4d707c 100644
@@ -622,8 +622,8 @@ void blk_mq_rq_timed_out(struct request *req, bool reserved)
 	}
 }
 
-static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
-		struct request *rq, void *priv, bool reserved)
+static bool blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, struct request *rq,
+				 void *priv, bool reserved)
 {
 	struct blk_mq_timeout_data *data = priv;
 

@@ -636,10 +636,10 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 			rq->errors = -EIO;
 			blk_mq_complete_request(rq);
 		}
-		return;
+		return false;
 	}
 	if (rq->cmd_flags & REQ_NO_TIMEOUT)
-		return;
+		return false;
 
 	if (time_after_eq(jiffies, rq->deadline)) {
 		if (!blk_mark_rq_complete(rq))

@@ -648,6 +648,8 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 		data->next = rq->deadline;
 		data->next_set = 1;
 	}
+
+	return false;
 }
 
 static void blk_mq_rq_timer(unsigned long priv)
drivers/block/nvme-core.c +4 -3
index e23be20a3417..46697ddfda82 100644
@@ -1260,7 +1260,7 @@ static void nvme_abort_req(struct request *req)
 	}
 }
 
-static void nvme_cancel_queue_ios(struct blk_mq_hw_ctx *hctx,
+static bool nvme_cancel_queue_ios(struct blk_mq_hw_ctx *hctx,
 				struct request *req, void *data, bool reserved)
 {
 	struct nvme_queue *nvmeq = data;

@@ -1270,12 +1270,12 @@ static void nvme_cancel_queue_ios(struct blk_mq_hw_ctx *hctx,
 	struct nvme_completion cqe;
 
 	if (!blk_mq_request_started(req))
-		return;
+		return false;
 
 	cmd = blk_mq_rq_to_pdu(req);
 
 	if (cmd->ctx == CMD_CTX_CANCELLED)
-		return;
+		return false;
 
 	if (blk_queue_dying(req->q))
 		cqe.status = cpu_to_le16((NVME_SC_ABORT_REQ | NVME_SC_DNR) << 1);

@@ -1287,6 +1287,7 @@ static void nvme_cancel_queue_ios(struct blk_mq_hw_ctx *hctx,
 						req->tag, nvmeq->qid);
 	ctx = cancel_cmd_info(cmd, &fn);
 	fn(nvmeq, ctx, &cqe);
+	return false;
 }
 
 static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
include/linux/blk-mq.h +1 -1
index 7aec86127335..b216deab80d9 100644
@@ -94,7 +94,7 @@ typedef int (init_request_fn)(void *, struct request *, unsigned int,
 typedef void (exit_request_fn)(void *, struct request *, unsigned int,
 		unsigned int);
 
-typedef void (busy_iter_fn)(struct blk_mq_hw_ctx *, struct request *, void *,
+typedef bool (busy_iter_fn)(struct blk_mq_hw_ctx *, struct request *, void *,
 		bool);
 
 struct blk_mq_ops {
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH 2/7 blk-mq: add helper to iterate all busy tags on all hardware queues

In reply to Jens Axboe

The current iter helper function deals with a hardware queue. Some callers want to access all queues, like the internal blk-mq timeout handling does. So add a helper for that.

Signed-off-by Jens Axboe <[email protected]>
---
 block/blk-mq.c         | 35 +++++++++++++++++++++++------------
 include/linux/blk-mq.h |  2 ++
 2 files changed, 25 insertions(+), 12 deletions(-)
block/blk-mq.c +23 -12
index 1cd34d4d707c..d8dcb71d3076 100644
@@ -561,6 +561,25 @@ void blk_mq_abort_requeue_list(struct request_queue *q)
 }
 EXPORT_SYMBOL(blk_mq_abort_requeue_list);
 
+void blk_mq_queue_busy_iter(struct request_queue *q, busy_iter_fn *fn,
+			    void *priv)
+{
+	struct blk_mq_hw_ctx *hctx;
+	int i;
+
+	queue_for_each_hw_ctx(q, hctx, i) {
+		/*
+		 * If no software queues are currently mapped to this
+		 * hardware queue, there's nothing to check
+		 */
+		if (!blk_mq_hw_queue_mapped(hctx))
+			continue;
+
+		blk_mq_tag_busy_iter(hctx, fn, priv);
+	}
+}
+EXPORT_SYMBOL_GPL(blk_mq_queue_busy_iter);
+
 static inline bool is_flush_request(struct request *rq,
 		struct blk_flush_queue *fq, unsigned int tag)
 {

@@ -659,24 +678,16 @@ static void blk_mq_rq_timer(unsigned long priv)
 		.next		= 0,
 		.next_set	= 0,
 	};
-	struct blk_mq_hw_ctx *hctx;
-	int i;
 
-	queue_for_each_hw_ctx(q, hctx, i) {
-		/*
-		 * If not software queues are currently mapped to this
-		 * hardware queue, there's nothing to check
-		 */
-		if (!blk_mq_hw_queue_mapped(hctx))
-			continue;
-
-		blk_mq_tag_busy_iter(hctx, blk_mq_check_expired, &data);
-	}
+	blk_mq_queue_busy_iter(q, blk_mq_check_expired, &data);
 
 	if (data.next_set) {
 		data.next = blk_rq_timeout(round_jiffies_up(data.next));
 		mod_timer(&q->timeout, data.next);
 	} else {
+		struct blk_mq_hw_ctx *hctx;
+		int i;
+
 		queue_for_each_hw_ctx(q, hctx, i)
 			blk_mq_tag_idle(hctx);
 	}
include/linux/blk-mq.h +2 -0
index b216deab80d9..8c010b5ea1b6 100644
@@ -219,6 +219,8 @@ void blk_mq_stop_hw_queues(struct request_queue *q);
 void blk_mq_start_hw_queues(struct request_queue *q);
 void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs);
+void blk_mq_queue_busy_iter(struct request_queue *q, busy_iter_fn *fn,
+			    void *priv);
 void blk_mq_tag_busy_iter(struct blk_mq_hw_ctx *hctx, busy_iter_fn *fn,
 		void *priv);
 void blk_mq_freeze_queue(struct request_queue *q);
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH 3/7 scsi: add scsi-mq helpers to retrieve pdu and check started state

In reply to Jens Axboe

scsi_mq_scmd_to_pdu() returns the LLD pdu associated with a struct scsi_cmnd.

scsi_mq_scmd_start() returns whether or not a given scsi command has been started or not.

Signed-off-by Jens Axboe <[email protected]>
---
 include/scsi/scsi_cmnd.h | 11 +++++++++++
 1 file changed, 11 insertions(+)
include/scsi/scsi_cmnd.h +11 -0
index 9fc1aecfc813..df02c89e8714 100644
@@ -8,6 +8,7 @@
 #include <linux/timer.h>
 #include <linux/scatterlist.h>
 #include <scsi/scsi_device.h>
+#include <linux/blk-mq.h>
 
 struct Scsi_Host;
 struct scsi_driver;

@@ -155,6 +156,16 @@ static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
 	return *(struct scsi_driver **)cmd->request->rq_disk->private_data;
 }
 
+static inline void *scsi_mq_scmd_to_pdu(struct scsi_cmnd *scmd)
+{
+	return blk_mq_rq_to_pdu(scmd->request) + sizeof(*scmd);
+}
+
+static inline bool scsi_mq_scmd_started(struct scsi_cmnd *scmd)
+{
+	return blk_mq_request_started(scmd->request);
+}
+
 extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
 extern void scsi_put_command(struct scsi_cmnd *);
 extern void scsi_finish_command(struct scsi_cmnd *cmd);
Signature · 1 line
1.9.1
#4 lore raw

Re: [PATCH 3/7] scsi: add scsi-mq helpers to retrieve pdu and check started state

include/scsi/scsi_cmnd.h static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd) · line 162

Quoted 4 lines · Jens Axboe
include/scsi/scsi_cmnd.h +4 -0
+static inline void *scsi_mq_scmd_to_pdu(struct scsi_cmnd *scmd)
+{
+	return blk_mq_rq_to_pdu(scmd->request) + sizeof(*scmd);
+}

We've already got scsi_cmd_priv to cover this.

include/scsi/scsi_cmnd.h static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd) · line 167

Quoted 4 lines
include/scsi/scsi_cmnd.h +4 -0
+static inline bool scsi_mq_scmd_started(struct scsi_cmnd *scmd)
+{
+	return blk_mq_request_started(scmd->request);
+}

I don't really think we want to expose this functionality, but more on that in my reply to the mpt2 patch.

author
#5 lore raw

PATCH 4/7 scsi: add scsi-mq helper for iterating over busy commands

In reply to Jens Axboe

This is basically just a wrapper around blk_mq_queue_busy_iter(), so that LLDs don't have to deal with or worry about blk-mq hardware queues.

Signed-off-by Jens Axboe <[email protected]>
---
 drivers/scsi/scsi_lib.c    | 25 +++++++++++++++++++++++++
 include/scsi/scsi_device.h |  3 +++
 2 files changed, 28 insertions(+)
drivers/scsi/scsi_lib.c +25 -0
index 54d7a6cbb98a..87a4c53c8b48 100644
@@ -648,6 +648,31 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 	}
 }
 
+struct scsi_mq_iter_data {
+	scsi_scmd_iter *fn;
+	void *priv;
+};
+
+static bool scsi_mq_iter_fn(struct blk_mq_hw_ctx *hctx, struct request *rq,
+			    void *priv, bool reserved)
+{
+	struct scsi_mq_iter_data *data = priv;
+	struct scsi_cmnd *scmd = rq->special;
+
+	return data->fn(scmd, data->priv);
+}
+
+void scsi_mq_scmd_busy_iter(struct scsi_device *sdev, scsi_scmd_iter *fn,
+			    void *priv)
+{
+	struct scsi_mq_iter_data data;
+
+	data.fn = fn;
+	data.priv = priv;
+	blk_mq_queue_busy_iter(sdev->request_queue, scsi_mq_iter_fn, &data);
+}
+EXPORT_SYMBOL(scsi_mq_scmd_busy_iter);
+
 /*
  * Function:    scsi_release_buffers()
  *
include/scsi/scsi_device.h +3 -0
index a4c9336811d1..9250b03613f2 100644
@@ -536,4 +536,7 @@ static inline int scsi_device_tpgs(struct scsi_device *sdev)
 	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
 #define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
 
+typedef bool (scsi_scmd_iter)(struct scsi_cmnd *, void *);
+void scsi_mq_scmd_busy_iter(struct scsi_device *sdev, scsi_scmd_iter *fn, void *priv);
+
 #endif /* _SCSI_SCSI_DEVICE_H */
Signature · 1 line
1.9.1
#6 lore raw

Re: [PATCH 4/7] scsi: add scsi-mq helper for iterating over busy commands

Quoted 3 lines · Jens Axboe

This is basically just a wrapper around blk_mq_queue_busy_iter(), so that LLDs don't have to deal with or worry about blk-mq hardware queues.

I'd prefer not to expose this to drivers, but if we do it needs to work for the !mq case.

author
#7 lore raw

PATCH 5/7 scsi: add host template init/exit_command hooks

In reply to Jens Axboe

If a LLD has hardware commands in the request pdu, then we need some helper hooks to help the driver initialize state at load time, and potentially to tear down state at rmmod time. Add a host template ->init_command() and ->exit_command() hook to help with that.

Signed-off-by Jens Axboe <[email protected]>
---
 drivers/scsi/scsi_lib.c  | 16 ++++++++++++++++
 include/scsi/scsi_host.h |  6 +++++-
 2 files changed, 21 insertions(+), 1 deletion(-)
drivers/scsi/scsi_lib.c +16 -0
index 87a4c53c8b48..d8f03526b836 100644
@@ -2081,11 +2081,23 @@ static int scsi_init_request(void *data, struct request *rq,
 		unsigned int numa_node)
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
+	struct Scsi_Host *shost = data;
 
 	cmd->sense_buffer = kzalloc_node(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL,
 			numa_node);
 	if (!cmd->sense_buffer)
 		return -ENOMEM;
+
+	if (shost->hostt->init_command) {
+		int ret;
+
+		ret = shost->hostt->init_command(shost, cmd, request_idx);
+		if (ret) {
+			kfree(cmd->sense_buffer);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 

@@ -2093,8 +2105,12 @@ static void scsi_exit_request(void *data, struct request *rq,
 		unsigned int hctx_idx, unsigned int request_idx)
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
+	struct Scsi_Host *shost = data;
 
 	kfree(cmd->sense_buffer);
+
+	if (shost->hostt->exit_command)
+		shost->hostt->exit_command(shost, cmd);
 }
 
 static u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
include/scsi/scsi_host.h +5 -1
index e113c757d555..baaa7a2fc07d 100644
@@ -496,10 +496,14 @@ struct scsi_host_template {
 	u64 vendor_id;
 
 	/*
-	 * Additional per-command data allocated for the driver.
+	 * Additional per-command data allocated for the driver, along
+	 * with init/exit helper hooks.
 	 */
 	unsigned int cmd_size;
 	struct scsi_host_cmd_pool *cmd_pool;
+	int (*init_command)(struct Scsi_Host *, struct scsi_cmnd *,
+				unsigned int);
+	void (*exit_command)(struct Scsi_Host *, struct scsi_cmnd *);
 
 	/* temporary flag to disable blk-mq I/O path */
 	bool disable_blk_mq;
Signature · 1 line
1.9.1
#8 lore raw

Re: [PATCH 5/7] scsi: add host template init/exit_command hooks

Quoted 6 lines · Jens Axboe

If a LLD has hardware commands in the request pdu, then we need some helper hooks to help the driver initialize state at load time, and potentially to tear down state at rmmod time. Add a host template ->init_command() and ->exit_command() hook to help with that.

Signed-off-by Jens Axboe <[email protected]>

Looks good in general, but please ensure these callsbacks also get invoked in the !mq case.

author
#9 lore raw

PATCH 6/7 mpt2sas: store scsi io tracker data in the scsi command / request

In reply to Jens Axboe

Instead of storing the IO tracker structure in a separate list that we need to pop/push to on every submit and complete (and lock), store it in the pdu associated with a request. This is possible on scsi-mq only, and further cuts the spinlock associated time for higher IOPS IO workloads. At 100K IOPS, this effectively cuts the locking time in half.

Signed-off-by Jens Axboe <[email protected]>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c  | 194 +++++++++++++++++++++++++----------
 drivers/scsi/mpt2sas/mpt2sas_base.h  |   3 +
 drivers/scsi/mpt2sas/mpt2sas_ctl.c   | 119 ++++++++++++++++-----
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |  89 ++++++++++++----
 4 files changed, 307 insertions(+), 98 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_base.c +139 -55
index 11248de92b3b..b0e4453af64c 100644
@@ -828,6 +828,20 @@ _base_async_event(struct MPT2SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	return;
 }
 
+struct scsiio_tracker *
+mpt2sas_get_st_from_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
+{
+	if (shost_use_blk_mq(ioc->shost)) {
+		struct scsi_cmnd *scmd;
+
+		scmd = scsi_mq_find_tag(ioc->shost, smid - 1);
+		if (!scmd)
+			return NULL;
+		return scsi_mq_scmd_to_pdu(scmd);
+	} else
+		return &ioc->scsi_lookup[smid - 1];
+}
+
 /**
  * _base_get_cb_idx - obtain the callback index
  * @ioc: per adapter object

@@ -842,8 +856,10 @@ _base_get_cb_idx(struct MPT2SAS_ADAPTER *ioc, u16 smid)
 	u8 cb_idx;
 
 	if (smid < ioc->hi_priority_smid) {
-		i = smid - 1;
-		cb_idx = ioc->scsi_lookup[i].cb_idx;
+		struct scsiio_tracker *st;
+
+		st = mpt2sas_get_st_from_smid(ioc, smid);
+		cb_idx = st->cb_idx;
 	} else if (smid < ioc->internal_smid) {
 		i = smid - ioc->hi_priority_smid;
 		cb_idx = ioc->hpr_lookup[i].cb_idx;

@@ -962,18 +978,17 @@ _base_interrupt(int irq, void *bus_id)
 			goto next;
 		if (smid) {
 			cb_idx = _base_get_cb_idx(ioc, smid);
-		if ((likely(cb_idx < MPT_MAX_CALLBACKS))
+			if ((likely(cb_idx < MPT_MAX_CALLBACKS))
 			    && (likely(mpt_callbacks[cb_idx] != NULL))) {
 				rc = mpt_callbacks[cb_idx](ioc, smid,
 				    msix_index, reply);
-			if (reply)
-				_base_display_reply_info(ioc, smid,
-				    msix_index, reply);
-			if (rc)
-				mpt2sas_base_free_smid(ioc, smid);
+				if (reply)
+					_base_display_reply_info(ioc, smid,
+							msix_index, reply);
+				if (rc)
+					mpt2sas_base_free_smid(ioc, smid);
 			}
-		}
-		if (!smid)
+		} else
 			_base_async_event(ioc, msix_index, reply);
 
 		/* reply free queue handling */

@@ -1724,6 +1739,18 @@ mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx,
 	struct scsiio_tracker *request;
 	u16 smid;
 
+	if (shost_use_blk_mq(ioc->shost)) {
+		/*
+		 * If we don't have a SCSI command associated with this smid,
+		 * bump it to high-prio
+		 */
+		if (!scmd)
+			return mpt2sas_base_get_smid_hpr(ioc, cb_idx);
+
+		request = scsi_mq_scmd_to_pdu(scmd);
+		return request->smid;
+	}
+
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	if (list_empty(&ioc->free_list)) {
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);

@@ -1771,6 +1798,31 @@ mpt2sas_base_get_smid_hpr(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx)
 	return smid;
 }
 
+static void
+_base_recovery_check(struct MPT2SAS_ADAPTER *ioc)
+{
+	/*
+	 * See _wait_for_commands_to_complete() call with regards to this code.
+	 */
+	if (ioc->shost_recovery && ioc->pending_io_count) {
+		if (ioc->pending_io_count == 1)
+			wake_up(&ioc->reset_wq);
+		ioc->pending_io_count = 0;
+	}
+}
+
+static void
+_dechain_st(struct MPT2SAS_ADAPTER *ioc, struct scsiio_tracker *st)
+{
+	struct chain_tracker *chain_req;
+
+	while (!list_empty(&st->chain_list)) {
+		chain_req = list_first_entry(&st->chain_list,
+						struct chain_tracker,
+						tracker_list);
+		list_move(&chain_req->tracker_list, &ioc->free_chain_list);
+	}
+}
 
 /**
  * mpt2sas_base_free_smid - put smid back on free_list

@@ -1784,20 +1836,32 @@ mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
 {
 	unsigned long flags;
 	int i;
-	struct chain_tracker *chain_req, *next;
+
+	if (shost_use_blk_mq(ioc->shost) && smid < ioc->hi_priority_smid) {
+		struct scsiio_tracker *st;
+
+		st = mpt2sas_get_st_from_smid(ioc, smid);
+		if (!st)
+			return;
+
+		st->direct_io = 0;
+
+		if (!list_empty(&st->chain_list)) {
+			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+			_dechain_st(ioc, st);
+			spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+		}
+
+		_base_recovery_check(ioc);
+		return;
+	}
 
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	if (smid < ioc->hi_priority_smid) {
 		/* scsiio queue */
 		i = smid - 1;
-		if (!list_empty(&ioc->scsi_lookup[i].chain_list)) {
-			list_for_each_entry_safe(chain_req, next,
-			    &ioc->scsi_lookup[i].chain_list, tracker_list) {
-				list_del_init(&chain_req->tracker_list);
-				list_add(&chain_req->tracker_list,
-				    &ioc->free_chain_list);
-			}
-		}
+		if (!list_empty(&ioc->scsi_lookup[i].chain_list))
+			_dechain_st(ioc, &ioc->scsi_lookup[i]);
 		ioc->scsi_lookup[i].cb_idx = 0xFF;
 		ioc->scsi_lookup[i].scmd = NULL;
 		ioc->scsi_lookup[i].direct_io = 0;

@@ -1805,15 +1869,7 @@ mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
 		    &ioc->free_list);
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
-		/*
-		 * See _wait_for_commands_to_complete() call with regards
-		 * to this code.
-		 */
-		if (ioc->shost_recovery && ioc->pending_io_count) {
-			if (ioc->pending_io_count == 1)
-				wake_up(&ioc->reset_wq);
-			ioc->pending_io_count--;
-		}
+		_base_recovery_check(ioc);
 		return;
 	} else if (smid < ioc->internal_smid) {
 		/* hi-priority */

@@ -2723,14 +2779,23 @@ _base_allocate_memory_pools(struct MPT2SAS_ADAPTER *ioc,  int sleep_flag)
 	    ioc->name, (unsigned long long) ioc->request_dma));
 	total_sz += sz;
 
-	sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
-	ioc->scsi_lookup_pages = get_order(sz);
-	ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
-	    GFP_KERNEL, ioc->scsi_lookup_pages);
-	if (!ioc->scsi_lookup) {
-		printk(MPT2SAS_ERR_FMT "scsi_lookup: get_free_pages failed, "
-		    "sz(%d)\n", ioc->name, (int)sz);
-		goto out;
+	/*
+	 * Don't need to allocate memory for scsiio_tracker array if we
+	 * are using scsi-mq, we embed it in the scsi_cmnd for that case.
+	 */
+	if (!shost_use_blk_mq(ioc->shost)) {
+		sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
+		ioc->scsi_lookup_pages = get_order(sz);
+		ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
+				    GFP_KERNEL, ioc->scsi_lookup_pages);
+		if (!ioc->scsi_lookup) {
+			printk(MPT2SAS_ERR_FMT "scsi_lookup: get_free_pages "
+				"failed, sz(%d)\n", ioc->name, (int)sz);
+			goto out;
+		}
+	} else {
+		ioc->scsi_lookup_pages = 0;
+		ioc->scsi_lookup = NULL;
 	}
 
 	dinitprintk(ioc, printk(MPT2SAS_INFO_FMT "scsiio(0x%p): "

@@ -4299,15 +4364,17 @@ _base_make_ioc_operational(struct MPT2SAS_ADAPTER *ioc, int sleep_flag)
 	/* initialize the scsi lookup free list */
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	INIT_LIST_HEAD(&ioc->free_list);
-	smid = 1;
-	for (i = 0; i < ioc->scsiio_depth; i++, smid++) {
-		INIT_LIST_HEAD(&ioc->scsi_lookup[i].chain_list);
-		ioc->scsi_lookup[i].cb_idx = 0xFF;
-		ioc->scsi_lookup[i].smid = smid;
-		ioc->scsi_lookup[i].scmd = NULL;
-		ioc->scsi_lookup[i].direct_io = 0;
-		list_add_tail(&ioc->scsi_lookup[i].tracker_list,
-		    &ioc->free_list);
+	if (!shost_use_blk_mq(ioc->shost)) {
+		smid = 1;
+		for (i = 0; i < ioc->scsiio_depth; i++, smid++) {
+			INIT_LIST_HEAD(&ioc->scsi_lookup[i].chain_list);
+			ioc->scsi_lookup[i].cb_idx = 0xFF;
+			ioc->scsi_lookup[i].smid = smid;
+			ioc->scsi_lookup[i].scmd = NULL;
+			ioc->scsi_lookup[i].direct_io = 0;
+			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
+					    &ioc->free_list);
+		}
 	}
 
 	/* hi-priority queue */

@@ -4772,7 +4839,7 @@ _wait_for_commands_to_complete(struct MPT2SAS_ADAPTER *ioc, int sleep_flag)
 {
 	u32 ioc_state;
 	unsigned long flags;
-	u16 i;
+	u16 i, pending, loops;
 
 	ioc->pending_io_count = 0;
 	if (sleep_flag != CAN_SLEEP)

@@ -4783,17 +4850,34 @@ _wait_for_commands_to_complete(struct MPT2SAS_ADAPTER *ioc, int sleep_flag)
 		return;
 
 	/* pending command count */
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	for (i = 0; i < ioc->scsiio_depth; i++)
-		if (ioc->scsi_lookup[i].cb_idx != 0xFF)
-			ioc->pending_io_count++;
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+	loops = 0;
+	do {
+		pending = 0;
+		spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+		for (i = 0; i < ioc->scsiio_depth; i++) {
+			struct scsiio_tracker *st;
+			struct scsi_cmnd *scmd;
+
+			if (shost_use_blk_mq(ioc->shost)) {
+				scmd = scsi_mq_find_tag(ioc->shost,  i);
+				if (scsi_mq_scmd_started(scmd))
+					pending++;
+			} else {
+				st = mpt2sas_get_st_from_smid(ioc, i + 1);
+				if (st->cb_idx != 0xFF)
+					pending++;
+			}
+		}
+		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
-	if (!ioc->pending_io_count)
-		return;
+		if (!pending)
+			break;
+
+		ioc->pending_io_count = 1;
 
-	/* wait for pending commands to complete */
-	wait_event_timeout(ioc->reset_wq, ioc->pending_io_count == 0, 10 * HZ);
+		/* wait for pending commands to complete */
+		wait_event_timeout(ioc->reset_wq, ioc->pending_io_count == 0, HZ);
+	} while (++loops <= 10);
 }
 
 /**
drivers/scsi/mpt2sas/mpt2sas_base.h +3 -0
index caff8d10cca4..cadb392126e0 100644
@@ -1045,6 +1045,9 @@ u16 mpt2sas_base_get_smid_hpr(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx);
 u16 mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx,
     struct scsi_cmnd *scmd);
 
+
+struct scsiio_tracker *mpt2sas_get_st_from_smid(struct MPT2SAS_ADAPTER *ioc,
+    u16 smid);
 u16 mpt2sas_base_get_smid(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx);
 void mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid);
 void mpt2sas_base_put_smid_scsi_io(struct MPT2SAS_ADAPTER *ioc, u16 smid,
drivers/scsi/mpt2sas/mpt2sas_ctl.c +94 -25

145 lines not on this page · raw message

drivers/scsi/mpt2sas/mpt2sas_scsih.c +71 -18

180 lines not on this page · raw message

Signature · 1 line
1.9.1
#10 lore raw

Re: [PATCH 6/7] mpt2sas: store scsi io tracker data in the scsi command / request

drivers/scsi/mpt2sas/mpt2sas_base.c _base_async_event(struct MPT2SAS_ADAPTER *ioc, u8 msix_index, u32 reply) · line 843

Quoted 13 lines · Jens Axboe
drivers/scsi/mpt2sas/mpt2sas_base.c +13 -0
+struct scsiio_tracker *
+mpt2sas_get_st_from_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
+{
+	if (shost_use_blk_mq(ioc->shost)) {
+		struct scsi_cmnd *scmd;
+
+		scmd = scsi_mq_find_tag(ioc->shost, smid - 1);
+		if (!scmd)
+			return NULL;
+		return scsi_mq_scmd_to_pdu(scmd);
+	} else
+		return &ioc->scsi_lookup[smid - 1];
+}

The mq case will also work for the !mq case when you call
scsi_host_find_tag and scsi_cmd_priv. In general all the mq-specific
codepathes you add should become the default and only one, even if this requires a lit bit of additional core work.

drivers/scsi/mpt2sas/mpt2sas_base.c mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx, · line 1748

Quoted 11 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +7 -0
@@ -1724,6 +1739,18 @@ mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx,
 	struct scsiio_tracker *request;
 	u16 smid;
 
+	if (shost_use_blk_mq(ioc->shost)) {
+		/*
+		 * If we don't have a SCSI command associated with this smid,
+		 * bump it to high-prio
+		 */
+		if (!scmd)
+			return mpt2sas_base_get_smid_hpr(ioc, cb_idx);

Seems like _ctl_do_mpt_command should be changed to just call mpt2sas_base_get_smid_hpr unconditionally instead of adding this hack Preferably as a standalone preparatory patch.

Quoted 8 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +5 -1
 	unsigned long flags;
 	int i;
-	struct chain_tracker *chain_req, *next;
+
+	if (shost_use_blk_mq(ioc->shost) && smid < ioc->hi_priority_smid) {
+		struct scsiio_tracker *st;
+
+		st = mpt2sas_get_st_from_smid(ioc, smid);

drivers/scsi/mpt2sas/mpt2sas_base.c mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid) · line 1853

Quoted 10 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +10 -0
+		if (!st)
+			return;
+
+		st->direct_io = 0;
+
+		if (!list_empty(&st->chain_list)) {
+			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+			_dechain_st(ioc, st);
+			spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+		}

This whole chain list thing looks bonkers to me. We always allocated a fixed multiple of the queue depth in ->chain_lookup, but then do this required list manipulation at least once per I/O submission and completion.

Seems like we should instead add an array of (cpu address, dma address) tuples to the scsiio_tracker and avoid all the chain_lookup / chain_list lookups entirely.

drivers/scsi/mpt2sas/mpt2sas_base.c _wait_for_commands_to_complete(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) · line 4864

Quoted 4 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +4 -0
+			if (shost_use_blk_mq(ioc->shost)) {
+				scmd = scsi_mq_find_tag(ioc->shost,  i);
+				if (scsi_mq_scmd_started(scmd))
+					pending++;

Ok, I guess we should move the request_started check into the _find_tag helpers, as tags that aren't started aren't something that driver should ever lookup.

drivers/scsi/mpt2sas/mpt2sas_ctl.c _ctl_poll(struct file *filep, poll_table *wait) · line 539

Quoted 8 lines
drivers/scsi/mpt2sas/mpt2sas_ctl.c +8 -0
+static bool
+_scmd_match(struct scsi_cmnd *scmd, u16 handle, u32 lun)
+{
+	struct MPT2SAS_DEVICE *priv_data;
+
+	if (scmd == NULL || scmd->device == NULL ||
+	    scmd->device->hostdata == NULL)
+		return false;

If the queue is started this can't ever happen.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (lun != scmd->device->lun)
+		return false;

If you pass in a specific scsi_device and thus request_queue this can't happen.

drivers/scsi/mpt2sas/mpt2sas_ctl.c _ctl_poll(struct file *filep, poll_table *wait) · line 618

Quoted 8 lines
drivers/scsi/mpt2sas/mpt2sas_ctl.c +8 -0
+static u16
+_ctl_find_smid(struct MPT2SAS_ADAPTER *ioc, u16 handle, u32 lun)
+{
+	if (shost_use_blk_mq(ioc->shost))
+		return _ctl_find_smid_mq(ioc, handle, lun);
+	else
+		return _ctl_find_smid_legacy(ioc, handle, lun);
+}

The caller of this looks entirely broken. It's a driver specific API to submit task management commands, duplicating the mid level code, and it doesn't even allow which task to target. I think we should just return a error when invoking MPI2_FUNCTION_SCSI_TASK_MGMT instead of digging us an even deeper grave here. If someone complains we'll have to find a way to redirect it to the generic EH ioctls.

maintainer
#11 lore raw

Re: [PATCH 6/7] mpt2sas: store scsi io tracker data in the scsi command / request

Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Jens Axboe
drivers/scsi/mpt2sas/mpt2sas_base.c +10 -0
+struct scsiio_tracker *
+mpt2sas_get_st_from_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
+{
+	if (shost_use_blk_mq(ioc->shost)) {
+		struct scsi_cmnd *scmd;
+
+		scmd = scsi_mq_find_tag(ioc->shost, smid - 1);
+		if (!scmd)
+			return NULL;
+		return scsi_mq_scmd_to_pdu(scmd);
Quoted 8 lines

drivers/scsi/mpt2sas/mpt2sas_base.c _base_async_event(struct MPT2SAS_ADAPTER *ioc, u8 msix_index, u32 reply) · line 843

Quoted 3 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +3 -0
+	} else
+		return &ioc->scsi_lookup[smid - 1];
+}

The mq case will also work for the !mq case when you call
scsi_host_find_tag and scsi_cmd_priv. In general all the mq-specific
codepathes you add should become the default and only one, even if this requires a lit bit of additional core work.

For the core code, I definitely agree. But for this case, in scsi-mq mode, we know that tag == smid - 1. That's not the case if we are not using scsi-mq.

In general, it'd be great if we could "convert" drivers and not have to support both scsi-mq and legacy mode. Then I could just rip the old code.

Quoted 4 lines
Quoted 4 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +0 -0
@@ -1724,6 +1739,18 @@ mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx,
  	struct scsiio_tracker *request;
  	u16 smid;
Quoted 11 lines

drivers/scsi/mpt2sas/mpt2sas_base.c mpt2sas_base_get_smid_scsiio(struct MPT2SAS_ADAPTER *ioc, u8 cb_idx, · line 1748

Quoted 7 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +7 -0
+	if (shost_use_blk_mq(ioc->shost)) {
+		/*
+		 * If we don't have a SCSI command associated with this smid,
+		 * bump it to high-prio
+		 */
+		if (!scmd)
+			return mpt2sas_base_get_smid_hpr(ioc, cb_idx);

Seems like _ctl_do_mpt_command should be changed to just call mpt2sas_base_get_smid_hpr unconditionally instead of adding this hack Preferably as a standalone preparatory patch.

Sounds reasonable, I'll do that.

Quoted 14 lines
Quoted 14 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +11 -1
  	unsigned long flags;
  	int i;
-	struct chain_tracker *chain_req, *next;
+
+	if (shost_use_blk_mq(ioc->shost) && smid < ioc->hi_priority_smid) {
+		struct scsiio_tracker *st;
+
+		st = mpt2sas_get_st_from_smid(ioc, smid);
+		if (!st)
+			return;
+
+		st->direct_io = 0;
+
+		if (!list_empty(&st->chain_list)) {
Quoted 8 lines

drivers/scsi/mpt2sas/mpt2sas_base.c mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid) · line 1853

Quoted 4 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +4 -0
+			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+			_dechain_st(ioc, st);
+			spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+		}

This whole chain list thing looks bonkers to me. We always allocated a fixed multiple of the queue depth in ->chain_lookup, but then do this required list manipulation at least once per I/O submission and completion.

It is completely crazy, and very suboptimal. The only thing that "saves" it is that we only need to do it multiple times for larger IOs, where a larger per-IO hit can be accepted. But yes, it really should just die a horrible death.

Quoted 3 lines

Seems like we should instead add an array of (cpu address, dma address) tuples to the scsiio_tracker and avoid all the chain_lookup / chain_list lookups entirely.

Agree.

Quoted 8 lines

drivers/scsi/mpt2sas/mpt2sas_base.c _wait_for_commands_to_complete(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) · line 4864

Quoted 4 lines
drivers/scsi/mpt2sas/mpt2sas_base.c +4 -0
+			if (shost_use_blk_mq(ioc->shost)) {
+				scmd = scsi_mq_find_tag(ioc->shost,  i);
+				if (scsi_mq_scmd_started(scmd))
+					pending++;

Ok, I guess we should move the request_started check into the _find_tag helpers, as tags that aren't started aren't something that driver should ever lookup.

I'll move it in there.

Quoted 17 lines

drivers/scsi/mpt2sas/mpt2sas_ctl.c _ctl_poll(struct file *filep, poll_table *wait) · line 539

Quoted 8 lines
drivers/scsi/mpt2sas/mpt2sas_ctl.c +8 -0
+static bool
+_scmd_match(struct scsi_cmnd *scmd, u16 handle, u32 lun)
+{
+	struct MPT2SAS_DEVICE *priv_data;
+
+	if (scmd == NULL || scmd->device == NULL ||
+	    scmd->device->hostdata == NULL)
+		return false;

If the queue is started this can't ever happen.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (lun != scmd->device->lun)
+		return false;

If you pass in a specific scsi_device and thus request_queue this can't happen.

Quoted 15 lines

drivers/scsi/mpt2sas/mpt2sas_ctl.c _ctl_poll(struct file *filep, poll_table *wait) · line 618

Quoted 8 lines
drivers/scsi/mpt2sas/mpt2sas_ctl.c +8 -0
+static u16
+_ctl_find_smid(struct MPT2SAS_ADAPTER *ioc, u16 handle, u32 lun)
+{
+	if (shost_use_blk_mq(ioc->shost))
+		return _ctl_find_smid_mq(ioc, handle, lun);
+	else
+		return _ctl_find_smid_legacy(ioc, handle, lun);
+}

The caller of this looks entirely broken. It's a driver specific API to submit task management commands, duplicating the mid level code, and it doesn't even allow which task to target. I think we should just return a error when invoking MPI2_FUNCTION_SCSI_TASK_MGMT instead of digging us an even deeper grave here. If someone complains we'll have to find a way to redirect it to the generic EH ioctls.

Sounds fine to me, will make my life a lot easier and we can kill this horrible lookup mess.

Signature · 1 line
Jens Axboe
#12 lore raw

Re: [PATCH 6/7] mpt2sas: store scsi io tracker data in the scsi command / request

Quoted 8 lines · Jens Axboe
Quoted 4 lines

The mq case will also work for the !mq case when you call
scsi_host_find_tag and scsi_cmd_priv. In general all the mq-specific
codepathes you add should become the default and only one, even if this requires a lit bit of additional core work.

For the core code, I definitely agree. But for this case, in scsi-mq mode, we know that tag == smid - 1. That's not the case if we are not using scsi-mq.

It is if you use the old block tagging code with host-wide tags. I guess you'll need to tell mpt2 and mpt3 to use that firs, though.

Quoted 2 lines

In general, it'd be great if we could "convert" drivers and not have to support both scsi-mq and legacy mode. Then I could just rip the old code.

Well, the whole point is that you should be able to write a driver like your mq version and it should just work. Even better would be if we could get rid of the old case entirely for scsi, but for that we need a I/O scheduler for blk-mq first :)

maintainer
#13 lore raw

Re: [PATCH 6/7] mpt2sas: store scsi io tracker data in the scsi command / request

Quoted 6 lines · Christoph Hellwig
Quoted 5 lines · Jens Axboe
Quoted 4 lines

The mq case will also work for the !mq case when you call
scsi_host_find_tag and scsi_cmd_priv. In general all the mq-specific
codepathes you add should become the default and only one, even if this requires a lit bit of additional core work.

Quoted 6 lines
Quoted 3 lines

For the core code, I definitely agree. But for this case, in scsi-mq mode, we know that tag == smid - 1. That's not the case if we are not using scsi-mq.

It is if you use the old block tagging code with host-wide tags. I guess you'll need to tell mpt2 and mpt3 to use that firs, though.

Yup

Quoted 5 lines
Quoted 2 lines

In general, it'd be great if we could "convert" drivers and not have to support both scsi-mq and legacy mode. Then I could just rip the old code.

Well, the whole point is that you should be able to write a driver like your mq version and it should just work. Even better would be if we

It'd be nice to have a host template flag that says "I only run scsi-mq", so we didn't have to cater to both cases. Or maybe that already exists and I just didn't look hard enough.

Quoted 2 lines

could get rid of the old case entirely for scsi, but for that we need a I/O scheduler for blk-mq first :)

I hear ya :)

Signature · 1 line
Jens Axboe
author
#14 lore raw

PATCH 7/7 mpt3sas: store scsi io tracker data in the scsi command / request

In reply to Jens Axboe

Instead of storing the IO tracker structure in a separate list that we need to pop/push to on every submit and complete (and lock), store it in the pdu associated with a request. This is possible on scsi-mq only, and further cuts the spinlock associated time for higher IOPS IO workloads. At 100K IOPS, this effectively cuts the locking time in half.

Signed-off-by Jens Axboe <[email protected]>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  | 180 +++++++++++++++++++++++++----------
 drivers/scsi/mpt3sas/mpt3sas_base.h  |   2 +
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   | 119 ++++++++++++++++++-----
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |  71 +++++++++++---
 4 files changed, 288 insertions(+), 84 deletions(-)
drivers/scsi/mpt3sas/mpt3sas_base.c +132 -48
index 14a781b6b88d..a4d287b7418b 100644
@@ -815,6 +815,20 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	return 1;
 }
 
+struct scsiio_tracker *
+mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
+{
+	if (shost_use_blk_mq(ioc->shost)) {
+		struct scsi_cmnd *scmd;
+
+		scmd = scsi_mq_find_tag(ioc->shost, smid - 1);
+		if (!scmd)
+			return NULL;
+		return scsi_mq_scmd_to_pdu(scmd);
+	} else
+		return &ioc->scsi_lookup[smid - 1];
+}
+
 /**
  * _base_get_cb_idx - obtain the callback index
  * @ioc: per adapter object

@@ -829,8 +843,10 @@ _base_get_cb_idx(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 	u8 cb_idx;
 
 	if (smid < ioc->hi_priority_smid) {
-		i = smid - 1;
-		cb_idx = ioc->scsi_lookup[i].cb_idx;
+		struct scsiio_tracker *st;
+
+		st = mpt3sas_get_st_from_smid(ioc, smid);
+		cb_idx = st->cb_idx;
 	} else if (smid < ioc->internal_smid) {
 		i = smid - ioc->hi_priority_smid;
 		cb_idx = ioc->hpr_lookup[i].cb_idx;

@@ -1176,6 +1192,7 @@ static struct chain_tracker *
 _base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
 	struct chain_tracker *chain_req;
+	struct scsiio_tracker *st;
 	unsigned long flags;
 
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);

@@ -1188,8 +1205,8 @@ _base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 	chain_req = list_entry(ioc->free_chain_list.next,
 	    struct chain_tracker, tracker_list);
 	list_del_init(&chain_req->tracker_list);
-	list_add_tail(&chain_req->tracker_list,
-	    &ioc->scsi_lookup[smid - 1].chain_list);
+	st = mpt3sas_get_st_from_smid(ioc, smid);
+	list_add_tail(&chain_req->tracker_list, &st->chain_list);
 	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 	return chain_req;
 }

@@ -2006,6 +2023,18 @@ mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
 	struct scsiio_tracker *request;
 	u16 smid;
 
+	if (shost_use_blk_mq(ioc->shost)) {
+		/*
+		 * If we don't have a SCSI command associated with this smid,
+		 * bump it to high-prio
+		 */
+		if (!scmd)
+			return mpt3sas_base_get_smid_hpr(ioc, cb_idx);
+
+		request = blk_mq_rq_to_pdu(scmd->request) + sizeof(*scmd);
+		return request->smid;
+	}
+
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	if (list_empty(&ioc->free_list)) {
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);

@@ -2053,6 +2082,32 @@ mpt3sas_base_get_smid_hpr(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx)
 	return smid;
 }
 
+static void
+_base_recovery_check(struct MPT3SAS_ADAPTER *ioc)
+{
+	/*
+	 * See _wait_for_commands_to_complete() call with regards to this code.
+	 */
+	if (ioc->shost_recovery && ioc->pending_io_count) {
+		if (ioc->pending_io_count == 1)
+			wake_up(&ioc->reset_wq);
+		ioc->pending_io_count = 0;
+	}
+}
+
+static void
+_dechain_st(struct MPT3SAS_ADAPTER *ioc, struct scsiio_tracker *st)
+{
+	struct chain_tracker *chain_req;
+
+	while (!list_empty(&st->chain_list)) {
+		chain_req = list_first_entry(&st->chain_list,
+						struct chain_tracker,
+						tracker_list);
+		list_move(&chain_req->tracker_list, &ioc->free_chain_list);
+	}
+}
+
 /**
  * mpt3sas_base_free_smid - put smid back on free_list
  * @ioc: per adapter object

@@ -2065,34 +2120,36 @@ mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
 	unsigned long flags;
 	int i;
-	struct chain_tracker *chain_req, *next;
+
+	if (shost_use_blk_mq(ioc->shost) && smid < ioc->hi_priority_smid) {
+		struct scsiio_tracker *st;
+
+		st = mpt3sas_get_st_from_smid(ioc, smid);
+		if (!st)
+			return;
+
+		if (!list_empty(&st->chain_list)) {
+			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+			_dechain_st(ioc, st);
+			spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+		}
+
+		_base_recovery_check(ioc);
+		return;
+	}
 
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	if (smid < ioc->hi_priority_smid) {
 		/* scsiio queue */
 		i = smid - 1;
-		if (!list_empty(&ioc->scsi_lookup[i].chain_list)) {
-			list_for_each_entry_safe(chain_req, next,
-			    &ioc->scsi_lookup[i].chain_list, tracker_list) {
-				list_del_init(&chain_req->tracker_list);
-				list_add(&chain_req->tracker_list,
-				    &ioc->free_chain_list);
-			}
-		}
+		if (!list_empty(&ioc->scsi_lookup[i].chain_list))
+			_dechain_st(ioc, &ioc->scsi_lookup[i]);
 		ioc->scsi_lookup[i].cb_idx = 0xFF;
 		ioc->scsi_lookup[i].scmd = NULL;
 		list_add(&ioc->scsi_lookup[i].tracker_list, &ioc->free_list);
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
-		/*
-		 * See _wait_for_commands_to_complete() call with regards
-		 * to this code.
-		 */
-		if (ioc->shost_recovery && ioc->pending_io_count) {
-			if (ioc->pending_io_count == 1)
-				wake_up(&ioc->reset_wq);
-			ioc->pending_io_count--;
-		}
+		_base_recovery_check(ioc);
 		return;
 	} else if (smid < ioc->internal_smid) {
 		/* hi-priority */

@@ -2896,14 +2953,23 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc,  int sleep_flag)
 	    ioc->name, (unsigned long long) ioc->request_dma));
 	total_sz += sz;
 
-	sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
-	ioc->scsi_lookup_pages = get_order(sz);
-	ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
-	    GFP_KERNEL, ioc->scsi_lookup_pages);
-	if (!ioc->scsi_lookup) {
-		pr_err(MPT3SAS_FMT "scsi_lookup: get_free_pages failed, sz(%d)\n",
-			ioc->name, (int)sz);
-		goto out;
+	/*
+	 * Don't need to allocate memory for scsiio_tracker array if we
+	 * are using scsi-mq, we embed it in the scsi_cmnd for that case.
+	 */
+	if (!shost_use_blk_mq(ioc->shost)) {
+		sz = ioc->scsiio_depth * sizeof(struct scsiio_tracker);
+		ioc->scsi_lookup_pages = get_order(sz);
+		ioc->scsi_lookup = (struct scsiio_tracker *)__get_free_pages(
+				    GFP_KERNEL, ioc->scsi_lookup_pages);
+		if (!ioc->scsi_lookup) {
+			pr_err(MPT3SAS_FMT "scsi_lookup: get_free_pages "
+					"failed, sz(%d)\n", ioc->name, (int)sz);
+			goto out;
+		}
+	} else {
+		ioc->scsi_lookup_pages = 0;
+		ioc->scsi_lookup = NULL;
 	}
 
 	dinitprintk(ioc, pr_info(MPT3SAS_FMT "scsiio(0x%p): depth(%d)\n",

@@ -4439,14 +4505,17 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
 	/* initialize the scsi lookup free list */
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	INIT_LIST_HEAD(&ioc->free_list);
-	smid = 1;
-	for (i = 0; i < ioc->scsiio_depth; i++, smid++) {
-		INIT_LIST_HEAD(&ioc->scsi_lookup[i].chain_list);
-		ioc->scsi_lookup[i].cb_idx = 0xFF;
-		ioc->scsi_lookup[i].smid = smid;
-		ioc->scsi_lookup[i].scmd = NULL;
-		list_add_tail(&ioc->scsi_lookup[i].tracker_list,
-		    &ioc->free_list);
+
+	if (!shost_use_blk_mq(ioc->shost)) {
+		smid = 1;
+		for (i = 0; i < ioc->scsiio_depth; i++, smid++) {
+			INIT_LIST_HEAD(&ioc->scsi_lookup[i].chain_list);
+			ioc->scsi_lookup[i].cb_idx = 0xFF;
+			ioc->scsi_lookup[i].smid = smid;
+			ioc->scsi_lookup[i].scmd = NULL;
+			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
+					    &ioc->free_list);
+		}
 	}
 
 	/* hi-priority queue */

@@ -4895,7 +4964,7 @@ _wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
 {
 	u32 ioc_state;
 	unsigned long flags;
-	u16 i;
+	u16 i, pending, loops;
 
 	ioc->pending_io_count = 0;
 	if (sleep_flag != CAN_SLEEP)

@@ -4906,17 +4975,32 @@ _wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
 		return;
 
 	/* pending command count */
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	for (i = 0; i < ioc->scsiio_depth; i++)
-		if (ioc->scsi_lookup[i].cb_idx != 0xFF)
-			ioc->pending_io_count++;
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+	loops = 0;
+	do {
+		pending = 0;
+		spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+		for (i = 0; i < ioc->scsiio_depth; i++) {
+			struct scsiio_tracker *st;
+			struct scsi_cmnd *scmd;
+
+			if (shost_use_blk_mq(ioc->shost)) {
+				scmd = scsi_mq_find_tag(ioc->shost, i);
+				if (scsi_mq_scmd_started(scmd))
+					pending++;
+			} else {
+				st = mpt3sas_get_st_from_smid(ioc, i + 1);
+				if (st->cb_idx != 0xFF)
+					pending++;
+			}
+		}
+		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
-	if (!ioc->pending_io_count)
-		return;
+		if (!pending)
+			break;
 
-	/* wait for pending commands to complete */
-	wait_event_timeout(ioc->reset_wq, ioc->pending_io_count == 0, 10 * HZ);
+		/* wait for pending commands to complete */
+		wait_event_timeout(ioc->reset_wq, ioc->pending_io_count == 0, HZ);
+	} while (++loops <= 10);
 }
 
 /**
drivers/scsi/mpt3sas/mpt3sas_base.h +2 -0
index afa881682bef..1a94791dbfe8 100644
@@ -986,6 +986,8 @@ u16 mpt3sas_base_get_smid_hpr(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
 u16 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
 	struct scsi_cmnd *scmd);
 
+struct scsiio_tracker *mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc,
+    u16 smid);
 u16 mpt3sas_base_get_smid(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
 void mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid);
 void mpt3sas_base_put_smid_scsi_io(struct MPT3SAS_ADAPTER *ioc, u16 smid,
drivers/scsi/mpt3sas/mpt3sas_ctl.c +94 -25

145 lines not on this page · raw message

drivers/scsi/mpt3sas/mpt3sas_scsih.c +60 -11

131 lines not on this page · raw message

Signature · 1 line
1.9.1