[PATCHv2 00/11] mpt3sas: Full mq support, part 1

[PATCHv2 00/11] mpt3sas: Full mq support, part 1

[Hannes Reinecke]

Hi all,

this is the first part of my patchset to enable scsi multiqueue for the
mpt3sas driver.
While the HBA only has a single mailbox register for submitting commands,
it does have individual receive queues per MSI-X interrupt and as such
does benefit from converting it to full multiqueue support.

On request from Broadcom the patchset has been split in two parts, one
to enable lockless command submission and converting to scsi-mq, and
the other one for exposing all hardware queues to the OS.

As usual, comments and reviews are welcome.

Changes to v1:
- Include reviews from Christoph
- Use reserved commands for ioctl passthrough commands
- Include reviews from Sreekanth


Hannes Reinecke (11):
  mpt3sas: switch to pci_alloc_irq_vectors
  mpt3sas: set default value for cb_idx
  mpt3sas: use 'list_splice_init()'
  mpt3sas: separate out _base_recovery_check()
  mpt3sas: open-code _scsih_scsi_lookup_get()
  mpt3sas: Introduce mpt3sas_get_st_from_smid()
  mpt3sas: check command status before attempting abort
  mpt3sas: always use smid 1 for ioctl passthrough
  mpt3sas: lockless command submission for scsi-mq
  scsi: allocate reserved commands
  mpt3sas: register reserved commands

 drivers/scsi/mpt3sas/mpt3sas_base.c      | 277 +++++++++---------
 drivers/scsi/mpt3sas/mpt3sas_base.h      |  17 +-
 drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  85 ++++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 482 ++++++++++++++-----------------
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  22 +-
 drivers/scsi/scsi_lib.c                  |   1 +
 include/scsi/scsi_host.h                 |   1 +
 7 files changed, 426 insertions(+), 459 deletions(-)

-- 
1.8.5.6

[PATCHv2 01/11] mpt3sas: switch to pci_alloc_irq_vectors

[Hannes Reinecke]

Cleanup the MSI-X handling allowing us to use
the PCI-layer provided vector allocation.

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 105 +++++++++++++++++-------------------
 drivers/scsi/mpt3sas/mpt3sas_base.h |   2 -
 2 files changed, 48 insertions(+), 59 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index a3fe1fb..5b7aec5 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1148,7 +1148,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 		/* TMs are on msix_index == 0 */
 		if (reply_q->msix_index == 0)
 			continue;
-		synchronize_irq(reply_q->vector);
+		synchronize_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index));
 	}
 }
 
@@ -1837,11 +1837,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 
 	list_for_each_entry_safe(reply_q, next, &ioc->reply_queue_list, list) {
 		list_del(&reply_q->list);
-		if (smp_affinity_enable) {
-			irq_set_affinity_hint(reply_q->vector, NULL);
-			free_cpumask_var(reply_q->affinity_hint);
-		}
-		free_irq(reply_q->vector, reply_q);
+		free_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index),
+			 reply_q);
 		kfree(reply_q);
 	}
 }
@@ -1850,13 +1847,13 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
  * _base_request_irq - request irq
  * @ioc: per adapter object
  * @index: msix index into vector table
- * @vector: irq vector
  *
  * Inserting respective reply_queue into the list.
  */
 static int
-_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
+_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index)
 {
+	struct pci_dev *pdev = ioc->pdev;
 	struct adapter_reply_queue *reply_q;
 	int r;
 
@@ -1868,14 +1865,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	}
 	reply_q->ioc = ioc;
 	reply_q->msix_index = index;
-	reply_q->vector = vector;
-
-	if (smp_affinity_enable) {
-		if (!zalloc_cpumask_var(&reply_q->affinity_hint, GFP_KERNEL)) {
-			kfree(reply_q);
-			return -ENOMEM;
-		}
-	}
 
 	atomic_set(&reply_q->busy, 0);
 	if (ioc->msix_enable)
@@ -1884,12 +1873,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	else
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
 		    ioc->driver_name, ioc->id);
-	r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
-	    reply_q);
+	r = request_irq(pci_irq_vector(pdev, index), _base_interrupt,
+			IRQF_SHARED, reply_q->name, reply_q);
 	if (r) {
 		pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
-		    reply_q->name, vector);
-		free_cpumask_var(reply_q->affinity_hint);
+		       reply_q->name, pci_irq_vector(pdev, index));
 		kfree(reply_q);
 		return -EBUSY;
 	}
@@ -1925,6 +1913,21 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	if (!nr_msix)
 		return;
 
+	if (smp_affinity_enable) {
+		list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
+			const cpumask_t *mask = pci_irq_get_affinity(ioc->pdev,
+							reply_q->msix_index);
+			if (!mask) {
+				pr_warn(MPT3SAS_FMT "no affinity for msi %x\n",
+					ioc->name, reply_q->msix_index);
+				continue;
+			}
+
+			for_each_cpu(cpu, mask)
+				ioc->cpu_msix_table[cpu] = reply_q->msix_index;
+		}
+		return;
+	}
 	cpu = cpumask_first(cpu_online_mask);
 
 	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
@@ -1938,18 +1941,9 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 			group++;
 
 		for (i = 0 ; i < group ; i++) {
-			ioc->cpu_msix_table[cpu] = index;
-			if (smp_affinity_enable)
-				cpumask_or(reply_q->affinity_hint,
-				   reply_q->affinity_hint, get_cpu_mask(cpu));
+			ioc->cpu_msix_table[cpu] = reply_q->msix_index;
 			cpu = cpumask_next(cpu, cpu_online_mask);
 		}
-		if (smp_affinity_enable)
-			if (irq_set_affinity_hint(reply_q->vector,
-					   reply_q->affinity_hint))
-				dinitprintk(ioc, pr_info(MPT3SAS_FMT
-				 "Err setting affinity hint to irq vector %d\n",
-				 ioc->name, reply_q->vector));
 		index++;
 	}
 }
@@ -1976,10 +1970,10 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 static int
 _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
 {
-	struct msix_entry *entries, *a;
 	int r;
 	int i, local_max_msix_vectors;
 	u8 try_msix = 0;
+	unsigned int irq_flags = PCI_IRQ_MSIX;
 
 	if (msix_disable == -1 || msix_disable == 0)
 		try_msix = 1;
@@ -1991,7 +1985,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 		goto try_ioapic;
 
 	ioc->reply_queue_count = min_t(int, ioc->cpu_count,
-	    ioc->msix_vector_count);
+		ioc->msix_vector_count);
 
 	printk(MPT3SAS_FMT "MSI-X vectors supported: %d, no of cores"
 	  ": %d, max_msix_vectors: %d\n", ioc->name, ioc->msix_vector_count,
@@ -2002,56 +1996,51 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	else
 		local_max_msix_vectors = max_msix_vectors;
 
-	if (local_max_msix_vectors > 0) {
+	if (local_max_msix_vectors > 0)
 		ioc->reply_queue_count = min_t(int, local_max_msix_vectors,
 			ioc->reply_queue_count);
-		ioc->msix_vector_count = ioc->reply_queue_count;
-	} else if (local_max_msix_vectors == 0)
+	else if (local_max_msix_vectors == 0)
 		goto try_ioapic;
 
 	if (ioc->msix_vector_count < ioc->cpu_count)
 		smp_affinity_enable = 0;
 
-	entries = kcalloc(ioc->reply_queue_count, sizeof(struct msix_entry),
-	    GFP_KERNEL);
-	if (!entries) {
-		dfailprintk(ioc, pr_info(MPT3SAS_FMT
-			"kcalloc failed @ at %s:%d/%s() !!!\n",
-			ioc->name, __FILE__, __LINE__, __func__));
-		goto try_ioapic;
-	}
+	if (smp_affinity_enable)
+		irq_flags |= PCI_IRQ_AFFINITY;
 
-	for (i = 0, a = entries; i < ioc->reply_queue_count; i++, a++)
-		a->entry = i;
-
-	r = pci_enable_msix_exact(ioc->pdev, entries, ioc->reply_queue_count);
-	if (r) {
+	r = pci_alloc_irq_vectors(ioc->pdev, 1, ioc->reply_queue_count,
+				  irq_flags);
+	if (r < 0) {
 		dfailprintk(ioc, pr_info(MPT3SAS_FMT
-			"pci_enable_msix_exact failed (r=%d) !!!\n",
+			"pci_alloc_irq_vectors failed (r=%d) !!!\n",
 			ioc->name, r));
-		kfree(entries);
 		goto try_ioapic;
 	}
 
 	ioc->msix_enable = 1;
-	for (i = 0, a = entries; i < ioc->reply_queue_count; i++, a++) {
-		r = _base_request_irq(ioc, i, a->vector);
+	ioc->reply_queue_count = r;
+	for (i = 0; i < ioc->reply_queue_count; i++) {
+		r = _base_request_irq(ioc, i);
 		if (r) {
 			_base_free_irq(ioc);
 			_base_disable_msix(ioc);
-			kfree(entries);
 			goto try_ioapic;
 		}
 	}
 
-	kfree(entries);
 	return 0;
 
 /* failback to io_apic interrupt routing */
  try_ioapic:
 
 	ioc->reply_queue_count = 1;
-	r = _base_request_irq(ioc, 0, ioc->pdev->irq);
+	r = pci_alloc_irq_vectors(ioc->pdev, 1, 1, PCI_IRQ_LEGACY);
+	if (r < 0) {
+		dfailprintk(ioc, pr_info(MPT3SAS_FMT
+			"pci_alloc_irq_vector(legacy) failed (r=%d) !!!\n",
+			ioc->name, r));
+	} else
+		r = _base_request_irq(ioc, 0);
 
 	return r;
 }
@@ -2222,7 +2211,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	list_for_each_entry(reply_q, &ioc->reply_queue_list, list)
 		pr_info(MPT3SAS_FMT "%s: IRQ %d\n",
 		    reply_q->name,  ((ioc->msix_enable) ? "PCI-MSI-X enabled" :
-		    "IO-APIC enabled"), reply_q->vector);
+		    "IO-APIC enabled"),
+		    pci_irq_vector(ioc->pdev, reply_q->msix_index));
 
 	pr_info(MPT3SAS_FMT "iomem(0x%016llx), mapped(0x%p), size(%d)\n",
 	    ioc->name, (unsigned long long)chip_phys, ioc->chip, memap_sz);
@@ -5357,7 +5347,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 		    sizeof(resource_size_t *), GFP_KERNEL);
 		if (!ioc->reply_post_host_index) {
 			dfailprintk(ioc, pr_info(MPT3SAS_FMT "allocation "
-				"for cpu_msix_table failed!!!\n", ioc->name));
+				"for reply_post_host_index failed!!!\n",
+				ioc->name));
 			r = -ENOMEM;
 			goto out_free_resources;
 		}
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index b2f3e24..1f460f2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -719,12 +719,10 @@ struct _event_ack_list {
 struct adapter_reply_queue {
 	struct MPT3SAS_ADAPTER	*ioc;
 	u8			msix_index;
-	unsigned int		vector;
 	u32			reply_post_host_index;
 	Mpi2ReplyDescriptorsUnion_t *reply_post_free;
 	char			name[MPT_NAME_LENGTH];
 	atomic_t		busy;
-	cpumask_var_t		affinity_hint;
 	struct list_head	list;
 };
 
-- 
1.8.5.6

[PATCHv2 02/11] mpt3sas: set default value for cb_idx

[Hannes Reinecke]

No functional change.

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 5b7aec5..3062171 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -873,7 +873,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 _base_get_cb_idx(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
 	int i;
-	u8 cb_idx;
+	u8 cb_idx = 0xFF;
 
 	if (smid < ioc->hi_priority_smid) {
 		i = smid - 1;
@@ -884,8 +884,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	} else if (smid <= ioc->hba_queue_depth) {
 		i = smid - ioc->internal_smid;
 		cb_idx = ioc->internal_lookup[i].cb_idx;
-	} else
-		cb_idx = 0xFF;
+	}
 	return cb_idx;
 }
 
-- 
1.8.5.6

[PATCHv2 03/11] mpt3sas: use 'list_splice_init()'

[Hannes Reinecke]

Use 'list_splice_init()' instead of hand-crafted function.
No functional change.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 3062171..f258d04 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2395,20 +2395,12 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 {
 	unsigned long flags;
 	int i;
-	struct chain_tracker *chain_req, *next;
 
 	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);
-			}
-		}
+		list_splice_init(&st->chain_list, &ioc->free_chain_list);
 		ioc->scsi_lookup[i].cb_idx = 0xFF;
 		ioc->scsi_lookup[i].scmd = NULL;
 		ioc->scsi_lookup[i].direct_io = 0;
-- 
1.8.5.6

Re: [PATCHv2 03/11] mpt3sas: use 'list_splice_init()'

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:02AM +0100, Hannes Reinecke wrote:
> Use 'list_splice_init()' instead of hand-crafted function.
> No functional change.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>

Looks fine,

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

[PATCHv2 04/11] mpt3sas: separate out _base_recovery_check()

[Hannes Reinecke]

No functional change.

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f258d04..6e0fc06 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2383,6 +2383,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	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--;
+	}
+}
+
 /**
  * mpt3sas_base_free_smid - put smid back on free_list
  * @ioc: per adapter object
@@ -2407,15 +2420,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 		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 */
-- 
1.8.5.6

[PATCHv2 05/11] mpt3sas: open-code _scsih_scsi_lookup_get()

[Hannes Reinecke]

Just a wrapper around the scsi lookup array and only used
in one place, so open-code it.

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index b8cd0a1..52de355 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1061,19 +1061,6 @@ struct _sas_node *
 }
 
 /**
- * _scsih_scsi_lookup_get - returns scmd entry
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Returns the smid stored scmd pointer.
- */
-static struct scsi_cmnd *
-_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
-{
-	return ioc->scsi_lookup[smid - 1].scmd;
-}
-
-/**
  * __scsih_scsi_lookup_get_clear - returns scmd entry without
  *						holding any lock.
  * @ioc: per adapter object
@@ -6126,7 +6113,7 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
 		if (ioc->shost_recovery)
 			goto out;
-		scmd = _scsih_scsi_lookup_get(ioc, smid);
+		scmd = ioc->scsi_lookup[smid - 1].scmd;
 		if (!scmd)
 			continue;
 		sdev = scmd->device;
-- 
1.8.5.6

[PATCHv2 06/11] mpt3sas: Introduce mpt3sas_get_st_from_smid()

[Hannes Reinecke]

Abstract accesses to the scsi_lookup array by introducing
mpt3sas_get_st_from_smid().

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c      | 20 ++++++++++++++++----
 drivers/scsi/mpt3sas/mpt3sas_base.h      |  2 ++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  4 +++-
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 6e0fc06..9b7273b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -862,6 +862,14 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	return 1;
 }
 
+struct scsiio_tracker *
+mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
+{
+	WARN_ON(!smid);
+	WARN_ON(smid >= ioc->hi_priority_smid);
+	return &ioc->scsi_lookup[smid - 1];
+}
+
 /**
  * _base_get_cb_idx - obtain the callback index
  * @ioc: per adapter object
@@ -876,8 +884,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	u8 cb_idx = 0xFF;
 
 	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);
+		if (st)
+			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;
@@ -1268,6 +1279,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 _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);
@@ -1280,8 +1292,8 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 	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;
 }
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 1f460f2..74186e3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1247,6 +1247,8 @@ __le32 mpt3sas_base_get_sense_buffer_dma(struct MPT3SAS_ADAPTER *ioc,
 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);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 52de355..a88f2ac 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2269,7 +2269,7 @@ struct _sas_node *
 	}
 
 	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK)
-		scsi_lookup = &ioc->scsi_lookup[smid_task - 1];
+		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
 
 	dtmprintk(ioc, pr_info(MPT3SAS_FMT
 		"sending tm: handle(0x%04x), task_type(0x%02x), smid(%d)\n",
diff --git a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
index 540bd50..06e3f7d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
@@ -270,7 +270,9 @@
 inline u8
 mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
-	return ioc->scsi_lookup[smid - 1].direct_io;
+	struct scsiio_tracker *st = mpt3sas_get_st_from_smid(ioc, smid);
+
+	return st ? st->direct_io : 0;
 }
 
 /**
-- 
1.8.5.6

Re: [PATCHv2 06/11] mpt3sas: Introduce mpt3sas_get_st_from_smid()

[Johannes Thumshirn]

On 02/17/2017 09:23 AM, Hannes Reinecke wrote:
> +struct scsiio_tracker *
> +mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
> +{
> +	WARN_ON(!smid);
> +	WARN_ON(smid >= ioc->hi_priority_smid);
> +	return &ioc->scsi_lookup[smid - 1];
> +}

Hmm if smid == 0 we'd be accessing &ioc->scsi_lookup[-1] and that's an
array out of bounds.

In patch 9/11 you'll get a unique tag of -2 (which is _not_ SCSI_NO_TAG)
and will be an array out of bounds as well in  blk_map_queue_find_tag()
or blk_mq_tag_to_rq().

if (WARN_ON(!smid))
	return NULL;

Should fix both.

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

Re: [PATCHv2 06/11] mpt3sas: Introduce mpt3sas_get_st_from_smid()

[Johannes Thumshirn]

On 02/17/2017 10:35 AM, Johannes Thumshirn wrote:
> On 02/17/2017 09:23 AM, Hannes Reinecke wrote:
>> +struct scsiio_tracker *
>> +mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>> +{
>> +	WARN_ON(!smid);
>> +	WARN_ON(smid >= ioc->hi_priority_smid);
>> +	return &ioc->scsi_lookup[smid - 1];
>> +}
> 
> Hmm if smid == 0 we'd be accessing &ioc->scsi_lookup[-1] and that's an
> array out of bounds.
> 
> In patch 9/11 you'll get a unique tag of -2 (which is _not_ SCSI_NO_TAG)
Of cause not you'll get -1 and this is SCSI_NO_TAG.

Not my day...


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

[PATCHv2 07/11] mpt3sas: check command status before attempting abort

[Hannes Reinecke]

When attempting a command abort we should check the command status
prior to sending the abort; the command might've been completed
already.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index a88f2ac..989cdc8 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2261,6 +2261,12 @@ struct _sas_node *
 		return (!rc) ? SUCCESS : FAILED;
 	}
 
+	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK) {
+		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
+		if (scsi_lookup->cb_idx == 0xFF)
+			return SUCCESS;
+	}
+
 	smid = mpt3sas_base_get_smid_hpr(ioc, ioc->tm_cb_idx);
 	if (!smid) {
 		pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
@@ -2268,9 +2274,6 @@ struct _sas_node *
 		return FAILED;
 	}
 
-	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK)
-		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
-
 	dtmprintk(ioc, pr_info(MPT3SAS_FMT
 		"sending tm: handle(0x%04x), task_type(0x%02x), smid(%d)\n",
 		ioc->name, handle, type, smid_task));
-- 
1.8.5.6

Re: [PATCHv2 07/11] mpt3sas: check command status before attempting abort

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:06AM +0100, Hannes Reinecke wrote:
> When attempting a command abort we should check the command status
> prior to sending the abort; the command might've been completed
> already.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index a88f2ac..989cdc8 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -2261,6 +2261,12 @@ struct _sas_node *
>  		return (!rc) ? SUCCESS : FAILED;
>  	}
>  
> +	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK) {
> +		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
> +		if (scsi_lookup->cb_idx == 0xFF)
> +			return SUCCESS;
> +	}

Not really new in this patch, but I'm worried that we don't check for
a NULL return here.  Are we 100% sure no invalid smid can be passed in?

But the patch itself looks fine:

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

Re: [PATCHv2 07/11] mpt3sas: check command status before attempting abort

[Hannes Reinecke]

On 02/17/2017 09:35 AM, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 09:23:06AM +0100, Hannes Reinecke wrote:
>> When attempting a command abort we should check the command status
>> prior to sending the abort; the command might've been completed
>> already.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> index a88f2ac..989cdc8 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> @@ -2261,6 +2261,12 @@ struct _sas_node *
>>  		return (!rc) ? SUCCESS : FAILED;
>>  	}
>>  
>> +	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK) {
>> +		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
>> +		if (scsi_lookup->cb_idx == 0xFF)
>> +			return SUCCESS;
>> +	}
> 
> Not really new in this patch, but I'm worried that we don't check for
> a NULL return here.  Are we 100% sure no invalid smid can be passed in?
> 
The check for that is being added in path 09/11; should've moved it to here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCHv2 08/11] mpt3sas: always use smid 1 for ioctl passthrough

[Hannes Reinecke]

ioctl passthrough commands require a SCSIIO smid, but cannot
easily integrate with the block layer. But as we're only ever
allowing one ioctl command at a time we can reserve smid 1
for ioctl commands.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 9b7273b..dec86c4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2355,13 +2355,20 @@ struct scsiio_tracker *
 		return 0;
 	}
 
-	request = list_entry(ioc->free_list.next,
-	    struct scsiio_tracker, tracker_list);
-	request->scmd = scmd;
+	if (!scmd) {
+		/* ioctl command, always use the first slot */
+		request = ioc->lookup[0];
+		request->scmd = NULL;
+	} else {
+		request = list_entry(ioc->free_list.next,
+				     struct scsiio_tracker, tracker_list);
+		request->scmd = scmd;
+	}
 	request->cb_idx = cb_idx;
 	smid = request->smid;
 	request->msix_io = _base_get_msix_index(ioc);
-	list_del(&request->tracker_list);
+	if (scmd)
+		list_del(&request->tracker_list);
 	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 	return smid;
 }
@@ -2429,7 +2436,9 @@ struct scsiio_tracker *
 		ioc->scsi_lookup[i].cb_idx = 0xFF;
 		ioc->scsi_lookup[i].scmd = NULL;
 		ioc->scsi_lookup[i].direct_io = 0;
-		list_add(&ioc->scsi_lookup[i].tracker_list, &ioc->free_list);
+		if (i > 0)
+			list_add(&ioc->scsi_lookup[i].tracker_list,
+				 &ioc->free_list);
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
 		_base_recovery_check(ioc);
@@ -5165,14 +5174,17 @@ struct scsiio_tracker *
 	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++) {
+	for (i = 1; 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 (i == 1)
+			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
+		else
+			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
+				      &ioc->free_list);
 	}
 
 	/* hi-priority queue */
-- 
1.8.5.6

Re: [PATCHv2 08/11] mpt3sas: always use smid 1 for ioctl passthrough

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:07AM +0100, Hannes Reinecke wrote:
> ioctl passthrough commands require a SCSIIO smid, but cannot
> easily integrate with the block layer. But as we're only ever
> allowing one ioctl command at a time we can reserve smid 1
> for ioctl commands.

I like the idea, but I don't think the implementation is correct.
More below.

> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2355,13 +2355,20 @@ struct scsiio_tracker *
>  		return 0;
>  	}
>  
> -	request = list_entry(ioc->free_list.next,
> -	    struct scsiio_tracker, tracker_list);
> -	request->scmd = scmd;
> +	if (!scmd) {
> +		/* ioctl command, always use the first slot */
> +		request = ioc->lookup[0];
> +		request->scmd = NULL;
> +	} else {
> +		request = list_entry(ioc->free_list.next,
> +				     struct scsiio_tracker, tracker_list);
> +		request->scmd = scmd;
> +	}
>  	request->cb_idx = cb_idx;
>  	smid = request->smid;
>  	request->msix_io = _base_get_msix_index(ioc);
> -	list_del(&request->tracker_list);
> +	if (scmd)
> +		list_del(&request->tracker_list);
>  	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
>  	return smid;

The freelist check just before the patch context and the locking
don't make much sense here.  I'd say just use a separate helper for the
ioctl smid, ala:

u16
mpt3sas_base_init_smid_pt(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx)
{
	struct scsiio_tracker *request = ioc->lookup[0];

	request->cb_idx = cb_idx;
	request->msix_io = _base_get_msix_index(ioc);
	return request->smid;
}

or given how trivial this is just open-code it in the caller.


>  		ioc->scsi_lookup[i].cb_idx = 0xFF;
>  		ioc->scsi_lookup[i].scmd = NULL;
>  		ioc->scsi_lookup[i].direct_io = 0;
> -		list_add(&ioc->scsi_lookup[i].tracker_list, &ioc->free_list);
> +		if (i > 0)
> +			list_add(&ioc->scsi_lookup[i].tracker_list,
> +				 &ioc->free_list);

Here we special case only request 0 as not added to the list.

> @@ -5165,14 +5174,17 @@ struct scsiio_tracker *
>  	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++) {
> +	for (i = 1; 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 (i == 1)
> +			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
> +		else
> +			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
> +				      &ioc->free_list);

And here we don't even iterate over smid 0, and then special case smid 1.

And note that the code in both loops is otherwise duplicated to start
with and could realy use a little helper to initialize the scsiio_tracker
structure.

Last but not least can_queue is initialized as:

        ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;

which doesn't take this new stolen smid into account.

Re: [PATCHv2 08/11] mpt3sas: always use smid 1 for ioctl passthrough

[Hannes Reinecke]

On 02/17/2017 09:45 AM, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 09:23:07AM +0100, Hannes Reinecke wrote:
>> ioctl passthrough commands require a SCSIIO smid, but cannot
>> easily integrate with the block layer. But as we're only ever
>> allowing one ioctl command at a time we can reserve smid 1
>> for ioctl commands.
> 
> I like the idea, but I don't think the implementation is correct.
> More below.
> 
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -2355,13 +2355,20 @@ struct scsiio_tracker *
>>  		return 0;
>>  	}
>>  
>> -	request = list_entry(ioc->free_list.next,
>> -	    struct scsiio_tracker, tracker_list);
>> -	request->scmd = scmd;
>> +	if (!scmd) {
>> +		/* ioctl command, always use the first slot */
>> +		request = ioc->lookup[0];
>> +		request->scmd = NULL;
>> +	} else {
>> +		request = list_entry(ioc->free_list.next,
>> +				     struct scsiio_tracker, tracker_list);
>> +		request->scmd = scmd;
>> +	}
>>  	request->cb_idx = cb_idx;
>>  	smid = request->smid;
>>  	request->msix_io = _base_get_msix_index(ioc);
>> -	list_del(&request->tracker_list);
>> +	if (scmd)
>> +		list_del(&request->tracker_list);
>>  	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
>>  	return smid;
> 
> The freelist check just before the patch context and the locking
> don't make much sense here.  I'd say just use a separate helper for the
> ioctl smid, ala:
> 
> u16
> mpt3sas_base_init_smid_pt(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx)
> {
> 	struct scsiio_tracker *request = ioc->lookup[0];
> 
> 	request->cb_idx = cb_idx;
> 	request->msix_io = _base_get_msix_index(ioc);
> 	return request->smid;
> }
> 
> or given how trivial this is just open-code it in the caller.
> 
> 
In principle, yes.
However, most of that is obsoleted when switching over to using reserved
commands in patch 11/11; then we properly register 'reserved' commands
with the block layer and this whole issue goes away.

>>  		ioc->scsi_lookup[i].cb_idx = 0xFF;
>>  		ioc->scsi_lookup[i].scmd = NULL;
>>  		ioc->scsi_lookup[i].direct_io = 0;
>> -		list_add(&ioc->scsi_lookup[i].tracker_list, &ioc->free_list);
>> +		if (i > 0)
>> +			list_add(&ioc->scsi_lookup[i].tracker_list,
>> +				 &ioc->free_list);
> 
> Here we special case only request 0 as not added to the list.
> 
>> @@ -5165,14 +5174,17 @@ struct scsiio_tracker *
>>  	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++) {
>> +	for (i = 1; 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 (i == 1)
>> +			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
>> +		else
>> +			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
>> +				      &ioc->free_list);
> 
> And here we don't even iterate over smid 0, and then special case smid 1.
> 
> And note that the code in both loops is otherwise duplicated to start
> with and could realy use a little helper to initialize the scsiio_tracker
> structure.
> 
Again, most of that code will be obsoleted with patch 09/11, which
switches over to scsi-mq.
This whole code is just a band-aid until the driver is switched over to
scsi-mq, where we have correct allocation for reserved commands.
I just couldn't be bothered to implement reserved commands for legacy
sq, seeing that it'll be obsoleted at some point in the future anyway.

> Last but not least can_queue is initialized as:
> 
>         ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;
> 
> which doesn't take this new stolen smid into account.
> 
Thing is, the internal commands are _never_ used anywhere, so
'can_queue' is actually still correct :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCHv2 09/11] mpt3sas: lockless command submission for scsi-mq

[Hannes Reinecke]

Enable lockless command submission for scsi-mq by moving the
command structure into the payload for struct request.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c      | 123 ++++-----
 drivers/scsi/mpt3sas/mpt3sas_base.h      |  13 +-
 drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  85 ++++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 460 ++++++++++++++-----------------
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  20 +-
 5 files changed, 330 insertions(+), 371 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index dec86c4..e9470a3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -865,9 +865,17 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 struct scsiio_tracker *
 mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
+	u32 unique_tag;
+	struct scsi_cmnd *cmd;
+
 	WARN_ON(!smid);
 	WARN_ON(smid >= ioc->hi_priority_smid);
-	return &ioc->scsi_lookup[smid - 1];
+	unique_tag = smid - 1;
+	cmd = scsi_host_find_tag(ioc->shost, unique_tag);
+	if (cmd)
+		return scsi_cmd_priv(cmd);
+
+	return NULL;
 }
 
 /**
@@ -2343,34 +2351,23 @@ struct scsiio_tracker *
 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
 	struct scsi_cmnd *scmd)
 {
-	unsigned long flags;
 	struct scsiio_tracker *request;
 	u16 smid;
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	if (list_empty(&ioc->free_list)) {
-		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-		pr_err(MPT3SAS_FMT "%s: smid not available\n",
-		    ioc->name, __func__);
-		return 0;
-	}
-
 	if (!scmd) {
-		/* ioctl command, always use the first slot */
-		request = ioc->lookup[0];
-		request->scmd = NULL;
+		smid = 1;
+		request = mpt3sas_get_st_from_smid(ioc, smid);
 	} else {
-		request = list_entry(ioc->free_list.next,
-				     struct scsiio_tracker, tracker_list);
-		request->scmd = scmd;
+		u32 unique_tag = blk_mq_unique_tag(scmd->request);
+		u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
+		request = scsi_cmd_priv(scmd);
+		smid = tag + 1;
 	}
 	request->cb_idx = cb_idx;
-	smid = request->smid;
 	request->msix_io = _base_get_msix_index(ioc);
-	if (scmd)
-		list_del(&request->tracker_list);
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-	return smid;
+	request->smid = smid;
+	INIT_LIST_HEAD(&request->chain_list);
+	return request->smid;
 }
 
 /**
@@ -2415,6 +2412,22 @@ struct scsiio_tracker *
 	}
 }
 
+void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
+			   struct scsiio_tracker *st)
+{
+	if (WARN_ON(st->smid == 0))
+		return;
+	st->cb_idx = 0xFF;
+	st->direct_io = 0;
+	if (!list_empty(&st->chain_list)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+		list_splice_init(&st->chain_list, &ioc->free_chain_list);
+		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+	}
+}
+
 /**
  * mpt3sas_base_free_smid - put smid back on free_list
  * @ioc: per adapter object
@@ -2428,22 +2441,21 @@ struct scsiio_tracker *
 	unsigned long flags;
 	int i;
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
 	if (smid < ioc->hi_priority_smid) {
-		/* scsiio queue */
-		i = smid - 1;
-		list_splice_init(&st->chain_list, &ioc->free_chain_list);
-		ioc->scsi_lookup[i].cb_idx = 0xFF;
-		ioc->scsi_lookup[i].scmd = NULL;
-		ioc->scsi_lookup[i].direct_io = 0;
-		if (i > 0)
-			list_add(&ioc->scsi_lookup[i].tracker_list,
-				 &ioc->free_list);
-		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+		struct scsiio_tracker *st;
 
+		st = mpt3sas_get_st_from_smid(ioc, smid);
+		if (WARN_ON(!st)) {
+			_base_recovery_check(ioc);
+			return;
+		}
+		mpt3sas_base_clear_st(ioc, st);
 		_base_recovery_check(ioc);
 		return;
-	} else if (smid < ioc->internal_smid) {
+	}
+
+	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
+	if (smid < ioc->internal_smid) {
 		/* hi-priority */
 		i = smid - ioc->hi_priority_smid;
 		ioc->hpr_lookup[i].cb_idx = 0xFF;
@@ -3276,10 +3288,6 @@ struct scsiio_tracker *
 		    ioc->config_page, ioc->config_page_dma);
 	}
 
-	if (ioc->scsi_lookup) {
-		free_pages((ulong)ioc->scsi_lookup, ioc->scsi_lookup_pages);
-		ioc->scsi_lookup = NULL;
-	}
 	kfree(ioc->hpr_lookup);
 	kfree(ioc->internal_lookup);
 	if (ioc->chain_lookup) {
@@ -3573,16 +3581,6 @@ struct scsiio_tracker *
 	    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;
-	}
-
 	dinitprintk(ioc, pr_info(MPT3SAS_FMT "scsiio(0x%p): depth(%d)\n",
 		ioc->name, ioc->request, ioc->scsiio_depth));
 
@@ -5170,22 +5168,7 @@ struct scsiio_tracker *
 		kfree(delayed_event_ack);
 	}
 
-	/* initialize the scsi lookup free list */
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	INIT_LIST_HEAD(&ioc->free_list);
-	smid = 1;
-	for (i = 1; 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;
-		if (i == 1)
-			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
-		else
-			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
-				      &ioc->free_list);
-	}
 
 	/* hi-priority queue */
 	INIT_LIST_HEAD(&ioc->hpr_free_list);
@@ -5685,6 +5668,13 @@ struct scsiio_tracker *
 	}
 }
 
+void _count_pending(struct request *req, void *data, bool reserved)
+{
+	struct MPT3SAS_ADAPTER *ioc = data;
+
+	ioc->pending_io_count++;
+}
+
 /**
  * _wait_for_commands_to_complete - reset controller
  * @ioc: Pointer to MPT_ADAPTER structure
@@ -5696,8 +5686,6 @@ struct scsiio_tracker *
 _wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
 {
 	u32 ioc_state;
-	unsigned long flags;
-	u16 i;
 
 	ioc->pending_io_count = 0;
 
@@ -5706,11 +5694,8 @@ struct scsiio_tracker *
 		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);
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
+				_count_pending, ioc);
 
 	if (!ioc->pending_io_count)
 		return;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 74186e3..8b38e49 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -646,19 +646,16 @@ struct chain_tracker {
 /**
  * struct scsiio_tracker - scsi mf request tracker
  * @smid: system message id
- * @scmd: scsi request pointer
  * @cb_idx: callback index
  * @direct_io: To indicate whether I/O is direct (WARPDRIVE)
- * @tracker_list: list of free request (ioc->free_list)
+ * @chain_list: list of associated firmware chain tracker
  * @msix_io: IO's msix
  */
 struct scsiio_tracker {
 	u16	smid;
-	struct scsi_cmnd *scmd;
 	u8	cb_idx;
 	u8	direct_io;
 	struct list_head chain_list;
-	struct list_head tracker_list;
 	u16     msix_io;
 };
 
@@ -1100,10 +1097,7 @@ struct MPT3SAS_ADAPTER {
 	u8		*request;
 	dma_addr_t	request_dma;
 	u32		request_dma_sz;
-	struct scsiio_tracker *scsi_lookup;
-	ulong		scsi_lookup_pages;
 	spinlock_t	scsi_lookup_lock;
-	struct list_head free_list;
 	int		pending_io_count;
 	wait_queue_head_t reset_wq;
 
@@ -1249,6 +1243,8 @@ 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);
+void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
+			   struct scsiio_tracker *st);
 
 u16 mpt3sas_base_get_smid(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx);
 void mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid);
@@ -1459,8 +1455,7 @@ void mpt3sas_init_warpdrive_properties(struct MPT3SAS_ADAPTER *ioc,
 mpt3sas_scsi_direct_io_set(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 direct_io);
 void
 mpt3sas_setup_direct_io(struct MPT3SAS_ADAPTER *ioc, struct scsi_cmnd *scmd,
-	struct _raid_device *raid_device, Mpi2SCSIIORequest_t *mpi_request,
-	u16 smid);
+	struct _raid_device *raid_device, Mpi2SCSIIORequest_t *mpi_request);
 
 /* NCQ Prio Handling Check */
 bool scsih_ncq_prio_supp(struct scsi_device *sdev);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 02fe1c4..e2a8c5d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -549,6 +549,58 @@ enum block_state {
 	return 0;
 }
 
+static bool
+_scmd_match(struct scsi_cmnd *scmd, u16 handle, u32 lun)
+{
+	struct MPT3SAS_DEVICE *priv_data;
+
+	if (scmd == NULL || scmd->device == NULL ||
+	    scmd->device->hostdata == NULL)
+		return false;
+	if (lun != scmd->device->lun)
+		return false;
+	priv_data = scmd->device->hostdata;
+	if (priv_data->sas_target == NULL)
+		return false;
+	if (priv_data->sas_target->handle != handle)
+		return false;
+
+	return true;
+}
+
+struct smid_match_data {
+	u16 handle;
+	u16 smid;
+	u32 lun;
+};
+
+static void
+_smid_fn(struct request *req, void *data, bool reserved)
+{
+	struct smid_match_data *smd = data;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
+	struct scsiio_tracker *st;
+
+	if (!_scmd_match(scmd, smd->handle, smd->lun))
+		return;
+
+	st = scsi_cmd_priv(scmd);
+	smd->smid = st->smid;
+}
+
+static u16
+_ctl_find_smid(struct MPT3SAS_ADAPTER *ioc, u16 handle, u32 lun)
+{
+	struct smid_match_data smd = {
+		.handle = handle,
+		.lun = lun,
+		.smid =  0,
+	};
+
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set, _smid_fn, &smd);
+	return smd.smid;
+}
+
 /**
  * _ctl_set_task_mid - assign an active smid to tm request
  * @ioc: per adapter object
@@ -562,12 +614,7 @@ enum block_state {
 _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
 	Mpi2SCSITaskManagementRequest_t *tm_request)
 {
-	u8 found = 0;
-	u16 i;
-	u16 handle;
-	struct scsi_cmnd *scmd;
-	struct MPT3SAS_DEVICE *priv_data;
-	unsigned long flags;
+	u16 smid, handle;
 	Mpi2SCSITaskManagementReply_t *tm_reply;
 	u32 sz;
 	u32 lun;
@@ -581,27 +628,11 @@ enum block_state {
 		return 0;
 
 	lun = scsilun_to_int((struct scsi_lun *)tm_request->LUN);
-
 	handle = le16_to_cpu(tm_request->DevHandle);
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	for (i = ioc->scsiio_depth; i && !found; i--) {
-		scmd = ioc->scsi_lookup[i - 1].scmd;
-		if (scmd == NULL || scmd->device == NULL ||
-		    scmd->device->hostdata == NULL)
-			continue;
-		if (lun != scmd->device->lun)
-			continue;
-		priv_data = scmd->device->hostdata;
-		if (priv_data->sas_target == NULL)
-			continue;
-		if (priv_data->sas_target->handle != handle)
-			continue;
-		tm_request->TaskMID = cpu_to_le16(ioc->scsi_lookup[i - 1].smid);
-		found = 1;
-	}
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
-	if (!found) {
+	smid = _ctl_find_smid(ioc, handle, lun);
+
+	if (!smid) {
 		dctlprintk(ioc, pr_info(MPT3SAS_FMT
 			"%s: handle(0x%04x), lun(%d), no active mid!!\n",
 			ioc->name,
@@ -621,6 +652,8 @@ enum block_state {
 		return 1;
 	}
 
+	tm_request->TaskMID = cpu_to_le16(smid);
+
 	dctlprintk(ioc, pr_info(MPT3SAS_FMT
 		"%s: handle(0x%04x), lun(%d), task_mid(%d)\n", ioc->name,
 	    desc, le16_to_cpu(tm_request->DevHandle), lun,
@@ -720,7 +753,7 @@ enum block_state {
 		}
 	} else {
 
-		smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL);
+		smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx);
 		if (!smid) {
 			pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
 			    ioc->name, __func__);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 989cdc8..b8e47da 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -147,7 +147,6 @@ static int _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 module_param(prot_mask, int, 0);
 MODULE_PARM_DESC(prot_mask, " host protection capabilities mask, def=7 ");
 
-
 /* raid transport support */
 struct raid_template *mpt3sas_raid_template;
 struct raid_template *mpt2sas_raid_template;
@@ -1061,7 +1060,7 @@ struct _sas_node *
 }
 
 /**
- * __scsih_scsi_lookup_get_clear - returns scmd entry without
+ * _scsih_scsi_lookup_get_clear - returns scmd entry without
  *						holding any lock.
  * @ioc: per adapter object
  * @smid: system request message index
@@ -1070,66 +1069,32 @@ struct _sas_node *
  * Then will dereference the stored scmd pointer.
  */
 static inline struct scsi_cmnd *
-__scsih_scsi_lookup_get_clear(struct MPT3SAS_ADAPTER *ioc,
-		u16 smid)
-{
-	struct scsi_cmnd *scmd = NULL;
-
-	swap(scmd, ioc->scsi_lookup[smid - 1].scmd);
-
-	return scmd;
-}
-
-/**
- * _scsih_scsi_lookup_get_clear - returns scmd entry
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Returns the smid stored scmd pointer.
- * Then will derefrence the stored scmd pointer.
- */
-static inline struct scsi_cmnd *
 _scsih_scsi_lookup_get_clear(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
-	unsigned long flags;
-	struct scsi_cmnd *scmd;
+	if (smid > 0) {
+		u32 unique_tag = smid - 1;
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	scmd = __scsih_scsi_lookup_get_clear(ioc, smid);
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-
-	return scmd;
+		return scsi_host_find_tag(ioc->shost, unique_tag);
+	}
+	return NULL;
 }
 
-/**
- * _scsih_scsi_lookup_find_by_scmd - scmd lookup
- * @ioc: per adapter object
- * @smid: system request message index
- * @scmd: pointer to scsi command object
- * Context: This function will acquire ioc->scsi_lookup_lock.
- *
- * This will search for a scmd pointer in the scsi_lookup array,
- * returning the revelent smid.  A returned value of zero means invalid.
- */
-static u16
-_scsih_scsi_lookup_find_by_scmd(struct MPT3SAS_ADAPTER *ioc, struct scsi_cmnd
-	*scmd)
+struct _scsih_scsi_lookup_data {
+	int channel;
+	int id;
+	int lun;
+	int result;
+};
+
+static void
+_scsih_scsi_target_lookup_fn(struct request *req, void *data, bool reserved)
 {
-	u16 smid;
-	unsigned long	flags;
-	int i;
+	struct _scsih_scsi_lookup_data *lookup_data = data;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	smid = 0;
-	for (i = 0; i < ioc->scsiio_depth; i++) {
-		if (ioc->scsi_lookup[i].scmd == scmd) {
-			smid = ioc->scsi_lookup[i].smid;
-			goto out;
-		}
-	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-	return smid;
+	if (scmd->device->id == lookup_data->id &&
+	    scmd->device->channel == lookup_data->channel)
+		lookup_data->result++;
 }
 
 /**
@@ -1146,23 +1111,26 @@ struct _sas_node *
 _scsih_scsi_lookup_find_by_target(struct MPT3SAS_ADAPTER *ioc, int id,
 	int channel)
 {
-	u8 found;
-	unsigned long	flags;
-	int i;
+	struct _scsih_scsi_lookup_data data = {
+		.channel = channel,
+		.id = id,
+		.result = 0,
+	};
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
+				_scsih_scsi_target_lookup_fn, &data);
+	return (data.result > 0);
+}
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	found = 0;
-	for (i = 0 ; i < ioc->scsiio_depth; i++) {
-		if (ioc->scsi_lookup[i].scmd &&
-		    (ioc->scsi_lookup[i].scmd->device->id == id &&
-		    ioc->scsi_lookup[i].scmd->device->channel == channel)) {
-			found = 1;
-			goto out;
-		}
-	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-	return found;
+static void
+_scsih_scsi_lun_lookup_fn(struct request *req, void *data, bool reserved)
+{
+	struct _scsih_scsi_lookup_data *lookup_data = data;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
+
+	if (scmd->device->id == lookup_data->id &&
+	    scmd->device->channel == lookup_data->channel &&
+	    scmd->device->lun == lookup_data->lun)
+		lookup_data->result++;
 }
 
 /**
@@ -1180,24 +1148,15 @@ struct _sas_node *
 _scsih_scsi_lookup_find_by_lun(struct MPT3SAS_ADAPTER *ioc, int id,
 	unsigned int lun, int channel)
 {
-	u8 found;
-	unsigned long	flags;
-	int i;
-
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	found = 0;
-	for (i = 0 ; i < ioc->scsiio_depth; i++) {
-		if (ioc->scsi_lookup[i].scmd &&
-		    (ioc->scsi_lookup[i].scmd->device->id == id &&
-		    ioc->scsi_lookup[i].scmd->device->channel == channel &&
-		    ioc->scsi_lookup[i].scmd->device->lun == lun)) {
-			found = 1;
-			goto out;
-		}
-	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-	return found;
+	struct _scsih_scsi_lookup_data data = {
+		.channel = channel,
+		.id = id,
+		.lun = lun,
+		.result = 0,
+	};
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
+				_scsih_scsi_lun_lookup_fn, &data);
+	return (data.result > 0);
 }
 
 /**
@@ -2263,6 +2222,8 @@ struct _sas_node *
 
 	if (type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK) {
 		scsi_lookup = mpt3sas_get_st_from_smid(ioc, smid_task);
+		if (!scsi_lookup)
+			return FAILED;
 		if (scsi_lookup->cb_idx == 0xFF)
 			return SUCCESS;
 	}
@@ -2331,7 +2292,7 @@ struct _sas_node *
 	switch (type) {
 	case MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK:
 		rc = SUCCESS;
-		if (scsi_lookup->scmd == NULL)
+		if (scsi_lookup->cb_idx == 0xFF)
 			break;
 		rc = FAILED;
 		break;
@@ -2451,7 +2412,7 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 {
 	struct MPT3SAS_ADAPTER *ioc = shost_priv(scmd->device->host);
 	struct MPT3SAS_DEVICE *sas_device_priv_data;
-	u16 smid;
+	struct scsiio_tracker *st;
 	u16 handle;
 	int r;
 
@@ -2470,8 +2431,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	}
 
 	/* search for the command */
-	smid = _scsih_scsi_lookup_find_by_scmd(ioc, scmd);
-	if (!smid) {
+	st = scsi_cmd_priv(scmd);
+	if (st->cb_idx == 0xff) {
 		scmd->result = DID_RESET << 16;
 		r = SUCCESS;
 		goto out;
@@ -2491,7 +2452,7 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	handle = sas_device_priv_data->sas_target->handle;
 	r = mpt3sas_scsih_issue_locked_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, scmd->device->lun,
-	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30);
+	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, st->smid, 30);
 
  out:
 	sdev_printk(KERN_INFO, scmd->device, "task abort: %s scmd(%p)\n",
@@ -3913,6 +3874,29 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
 }
 
+void _flush_running(struct request *req, void *data, bool reserved)
+{
+	struct MPT3SAS_ADAPTER *ioc = data;
+	struct scsi_cmnd *scmd;
+	struct scsiio_tracker *st;
+
+	scmd = blk_mq_rq_to_pdu(req);
+	st = scsi_cmd_priv(scmd);
+	if (ata_12_16_cmd(scmd))
+		scsi_internal_device_unblock(scmd->device,
+					     SDEV_RUNNING);
+	mpt3sas_base_clear_st(ioc, st);
+	scsi_dma_unmap(scmd);
+
+	if (ioc->pci_error_recovery)
+		scmd->result = DID_NO_CONNECT << 16;
+	else
+		scmd->result = DID_RESET << 16;
+	scmd->scsi_done(scmd);
+
+	ioc->pending_io_count++;
+}
+
 /**
  * _scsih_flush_running_cmds - completing outstanding commands.
  * @ioc: per adapter object
@@ -3925,28 +3909,11 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 static void
 _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
 {
-	struct scsi_cmnd *scmd;
-	u16 smid;
-	u16 count = 0;
-
-	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
-		scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
-		if (!scmd)
-			continue;
-		count++;
-		if (ata_12_16_cmd(scmd))
-			scsi_internal_device_unblock(scmd->device,
-							SDEV_RUNNING);
-		mpt3sas_base_free_smid(ioc, smid);
-		scsi_dma_unmap(scmd);
-		if (ioc->pci_error_recovery)
-			scmd->result = DID_NO_CONNECT << 16;
-		else
-			scmd->result = DID_RESET << 16;
-		scmd->scsi_done(scmd);
-	}
+	ioc->pending_io_count = 0;
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
+				_flush_running, ioc);
 	dtmprintk(ioc, pr_info(MPT3SAS_FMT "completing %d cmds\n",
-	    ioc->name, count));
+	    ioc->name, ioc->pending_io_count));
 }
 
 /**
@@ -4180,8 +4147,7 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 
 	raid_device = sas_target_priv_data->raid_device;
 	if (raid_device && raid_device->direct_io_enabled)
-		mpt3sas_setup_direct_io(ioc, scmd, raid_device, mpi_request,
-		    smid);
+		mpt3sas_setup_direct_io(ioc, scmd, raid_device, mpi_request);
 
 	if (likely(mpi_request->Function == MPI2_FUNCTION_SCSI_IO_REQUEST)) {
 		if (sas_target_priv_data->flags & MPT_TARGET_FASTPATH_IO) {
@@ -4652,16 +4618,10 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	u32 log_info;
 	struct MPT3SAS_DEVICE *sas_device_priv_data;
 	u32 response_code = 0;
-	unsigned long flags;
 
 	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
 
-	if (ioc->broadcast_aen_busy || ioc->pci_error_recovery ||
-			ioc->got_task_abort_from_ioctl)
-		scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
-	else
-		scmd = __scsih_scsi_lookup_get_clear(ioc, smid);
-
+	scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
 	if (scmd == NULL)
 		return 1;
 
@@ -4690,10 +4650,9 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	if (mpt3sas_scsi_direct_io_get(ioc, smid) &&
 	     ((ioc_status & MPI2_IOCSTATUS_MASK)
 	      != MPI2_IOCSTATUS_SCSI_TASK_TERMINATED)) {
-		spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-		ioc->scsi_lookup[smid - 1].scmd = scmd;
-		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-		mpt3sas_scsi_direct_io_set(ioc, smid, 0);
+		struct scsiio_tracker *st = scsi_cmd_priv(scmd);
+
+		st->direct_io = 0;
 		memcpy(mpi_request->CDB.CDB32, scmd->cmnd, scmd->cmd_len);
 		mpi_request->DevHandle =
 		    cpu_to_le16(sas_device_priv_data->sas_target->handle);
@@ -4863,11 +4822,12 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 		_scsih_scsi_ioc_info(ioc , scmd, mpi_reply, smid);
 
  out:
+	mpt3sas_base_clear_st(ioc, scsi_cmd_priv(scmd));
 
 	scsi_dma_unmap(scmd);
 
 	scmd->scsi_done(scmd);
-	return 1;
+	return 0;
 }
 
 /**
@@ -6061,6 +6021,108 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 		     fw_event->event_data);
 }
 
+struct _abort_sas_task_data {
+	struct MPT3SAS_ADAPTER *ioc;
+	int query_count;
+	int retry;
+	int termination_count;
+};
+
+void _abort_sas_task(struct request *req, void *data, bool reserved)
+{
+	struct _abort_sas_task_data *priv = data;
+	struct scsi_cmnd *scmd;
+	struct scsi_device *sdev;
+	u16 handle;
+	u32 lun;
+	struct scsiio_tracker *st;
+	struct MPT3SAS_DEVICE *sas_device_priv_data;
+	Mpi2SCSITaskManagementReply_t *mpi_reply;
+	u16 ioc_status;
+	int r;
+	u8 task_abort_retries;
+
+	scmd = blk_mq_rq_to_pdu(req);
+	st = scsi_cmd_priv(scmd);
+
+	sdev = scmd->device;
+	sas_device_priv_data = sdev->hostdata;
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target)
+		return;
+	/* skip hidden raid components */
+	if (sas_device_priv_data->sas_target->flags &
+	    MPT_TARGET_FLAGS_RAID_COMPONENT)
+		return;
+	/* skip volumes */
+	if (sas_device_priv_data->sas_target->flags &
+	    MPT_TARGET_FLAGS_VOLUME)
+		return;
+
+	handle = sas_device_priv_data->sas_target->handle;
+	lun = sas_device_priv_data->lun;
+	mpi_reply = priv->ioc->tm_cmds.reply;
+	priv->query_count++;
+
+	if (priv->ioc->shost_recovery)
+		return;
+
+	r = mpt3sas_scsih_issue_tm(priv->ioc, handle, 0, 0, lun,
+		MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, st->smid, 30);
+	if (r == FAILED) {
+		sdev_printk(KERN_WARNING, sdev,
+			    "mpt3sas_scsih_issue_tm: FAILED when sending "
+			    "QUERY_TASK: scmd(%p)\n", scmd);
+		priv->retry++;
+		return;
+	}
+	ioc_status = le16_to_cpu(mpi_reply->IOCStatus)
+		& MPI2_IOCSTATUS_MASK;
+	if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
+		sdev_printk(KERN_WARNING, sdev,
+			"query task: FAILED with IOCSTATUS(0x%04x), scmd(%p)\n",
+			ioc_status, scmd);
+		priv->retry++;
+		return;
+	}
+
+	/* see if IO is still owned by IOC and target */
+	if (mpi_reply->ResponseCode ==
+	    MPI2_SCSITASKMGMT_RSP_TM_SUCCEEDED ||
+	    mpi_reply->ResponseCode ==
+	    MPI2_SCSITASKMGMT_RSP_IO_QUEUED_ON_IOC)
+		return;
+
+	task_abort_retries = 0;
+ tm_retry:
+	if (task_abort_retries++ == 60) {
+		dewtprintk(priv->ioc, pr_info(MPT3SAS_FMT
+			"%s: ABORT_TASK: giving up\n",
+			priv->ioc->name, __func__));
+		priv->retry++;
+		return;
+	}
+
+	if (priv->ioc->shost_recovery)
+		return;
+
+	r = mpt3sas_scsih_issue_tm(priv->ioc, handle, sdev->channel, sdev->id,
+		sdev->lun, MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, st->smid, 30);
+	if (r == FAILED) {
+		sdev_printk(KERN_WARNING, sdev,
+			    "mpt3sas_scsih_issue_tm: ABORT_TASK: FAILED : "
+			    "scmd(%p)\n", scmd);
+		goto tm_retry;
+	}
+
+	if (task_abort_retries > 1)
+		sdev_printk(KERN_WARNING, sdev,
+			    "mpt3sas_scsih_issue_tm: ABORT_TASK: RETRIES (%d):"
+			    " scmd(%p)\n",
+			    task_abort_retries - 1, scmd);
+
+	priv->termination_count += le32_to_cpu(mpi_reply->TerminationCount);
+}
+
 /**
  * _scsih_sas_broadcast_primitive_event - handle broadcast events
  * @ioc: per adapter object
@@ -6073,22 +6135,17 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 _scsih_sas_broadcast_primitive_event(struct MPT3SAS_ADAPTER *ioc,
 	struct fw_event_work *fw_event)
 {
-	struct scsi_cmnd *scmd;
-	struct scsi_device *sdev;
-	u16 smid, handle;
-	u32 lun;
-	struct MPT3SAS_DEVICE *sas_device_priv_data;
-	u32 termination_count;
-	u32 query_count;
-	Mpi2SCSITaskManagementReply_t *mpi_reply;
 	Mpi2EventDataSasBroadcastPrimitive_t *event_data =
 		(Mpi2EventDataSasBroadcastPrimitive_t *)
 		fw_event->event_data;
-	u16 ioc_status;
-	unsigned long flags;
-	int r;
 	u8 max_retries = 0;
-	u8 task_abort_retries;
+	struct _abort_sas_task_data priv = {
+		.ioc = ioc,
+		.retry = 0,
+		.query_count = 0,
+		.termination_count = 0,
+	};
+
 
 	mutex_lock(&ioc->tm_cmds.mutex);
 	pr_info(MPT3SAS_FMT
@@ -6098,123 +6155,23 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 
 	_scsih_block_io_all_device(ioc);
 
-	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	mpi_reply = ioc->tm_cmds.reply;
- broadcast_aen_retry:
-
-	/* sanity checks for retrying this loop */
+retry_iter:
 	if (max_retries++ == 5) {
 		dewtprintk(ioc, pr_info(MPT3SAS_FMT "%s: giving up\n",
-		    ioc->name, __func__));
+					ioc->name, __func__));
 		goto out;
 	} else if (max_retries > 1)
 		dewtprintk(ioc, pr_info(MPT3SAS_FMT "%s: %d retry\n",
-		    ioc->name, __func__, max_retries - 1));
-
-	termination_count = 0;
-	query_count = 0;
-	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
-		if (ioc->shost_recovery)
-			goto out;
-		scmd = ioc->scsi_lookup[smid - 1].scmd;
-		if (!scmd)
-			continue;
-		sdev = scmd->device;
-		sas_device_priv_data = sdev->hostdata;
-		if (!sas_device_priv_data || !sas_device_priv_data->sas_target)
-			continue;
-		 /* skip hidden raid components */
-		if (sas_device_priv_data->sas_target->flags &
-		    MPT_TARGET_FLAGS_RAID_COMPONENT)
-			continue;
-		 /* skip volumes */
-		if (sas_device_priv_data->sas_target->flags &
-		    MPT_TARGET_FLAGS_VOLUME)
-			continue;
-
-		handle = sas_device_priv_data->sas_target->handle;
-		lun = sas_device_priv_data->lun;
-		query_count++;
-
-		if (ioc->shost_recovery)
-			goto out;
-
-		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-		r = mpt3sas_scsih_issue_tm(ioc, handle, 0, 0, lun,
-		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, smid, 30);
-		if (r == FAILED) {
-			sdev_printk(KERN_WARNING, sdev,
-			    "mpt3sas_scsih_issue_tm: FAILED when sending "
-			    "QUERY_TASK: scmd(%p)\n", scmd);
-			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-			goto broadcast_aen_retry;
-		}
-		ioc_status = le16_to_cpu(mpi_reply->IOCStatus)
-		    & MPI2_IOCSTATUS_MASK;
-		if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
-			sdev_printk(KERN_WARNING, sdev,
-				"query task: FAILED with IOCSTATUS(0x%04x), scmd(%p)\n",
-				ioc_status, scmd);
-			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-			goto broadcast_aen_retry;
-		}
-
-		/* see if IO is still owned by IOC and target */
-		if (mpi_reply->ResponseCode ==
-		     MPI2_SCSITASKMGMT_RSP_TM_SUCCEEDED ||
-		     mpi_reply->ResponseCode ==
-		     MPI2_SCSITASKMGMT_RSP_IO_QUEUED_ON_IOC) {
-			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-			continue;
-		}
-		task_abort_retries = 0;
- tm_retry:
-		if (task_abort_retries++ == 60) {
-			dewtprintk(ioc, pr_info(MPT3SAS_FMT
-			    "%s: ABORT_TASK: giving up\n", ioc->name,
-			    __func__));
-			spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-			goto broadcast_aen_retry;
-		}
-
-		if (ioc->shost_recovery)
-			goto out_no_lock;
-
-		r = mpt3sas_scsih_issue_tm(ioc, handle, sdev->channel, sdev->id,
-		    sdev->lun, MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid,
-		    30);
-		if (r == FAILED) {
-			sdev_printk(KERN_WARNING, sdev,
-			    "mpt3sas_scsih_issue_tm: ABORT_TASK: FAILED : "
-			    "scmd(%p)\n", scmd);
-			goto tm_retry;
-		}
-
-		if (task_abort_retries > 1)
-			sdev_printk(KERN_WARNING, sdev,
-			    "mpt3sas_scsih_issue_tm: ABORT_TASK: RETRIES (%d):"
-			    " scmd(%p)\n",
-			    task_abort_retries - 1, scmd);
-
-		termination_count += le32_to_cpu(mpi_reply->TerminationCount);
-		spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	}
-
-	if (ioc->broadcast_aen_pending) {
-		dewtprintk(ioc, pr_info(MPT3SAS_FMT
-			"%s: loop back due to pending AEN\n",
-			ioc->name, __func__));
-		 ioc->broadcast_aen_pending = 0;
-		 goto broadcast_aen_retry;
-	}
-
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
- out_no_lock:
+				ioc->name, __func__, max_retries - 1));
 
+	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
+				_abort_sas_task, &priv);
+	if (priv.retry)
+		goto retry_iter;
+out:
 	dewtprintk(ioc, pr_info(MPT3SAS_FMT
 	    "%s - exit, query_count = %d termination_count = %d\n",
-	    ioc->name, __func__, query_count, termination_count));
+	    ioc->name, __func__, priv.query_count, priv.termination_count));
 
 	ioc->broadcast_aen_busy = 0;
 	if (!ioc->shost_recovery)
@@ -8615,6 +8572,7 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER *ioc,
 	.shost_attrs			= mpt3sas_host_attrs,
 	.sdev_attrs			= mpt3sas_dev_attrs,
 	.track_queue_depth		= 1,
+	.cmd_size			= sizeof(struct scsiio_tracker),
 };
 
 /* raid transport support for SAS 2.0 HBA devices */
@@ -8653,6 +8611,7 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER *ioc,
 	.shost_attrs			= mpt3sas_host_attrs,
 	.sdev_attrs			= mpt3sas_dev_attrs,
 	.track_queue_depth		= 1,
+	.cmd_size			= sizeof(struct scsiio_tracker),
 };
 
 /* raid transport support for SAS 3.0 HBA devices */
@@ -8864,6 +8823,7 @@ static void sas_device_make_active(struct MPT3SAS_ADAPTER *ioc,
 	shost->max_lun = max_lun;
 	shost->transportt = mpt3sas_transport_template;
 	shost->unique_id = ioc->id;
+	shost->use_blk_mq = 1;
 
 	if (max_sectors != 0xFFFF) {
 		if (max_sectors < 64) {
diff --git a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
index 06e3f7d..f9fd0bc 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
@@ -276,20 +276,6 @@
 }
 
 /**
- * mpt3sas_scsi_direct_io_set - sets direct io flag
- * @ioc: per adapter object
- * @smid: system request message index
- * @direct_io: Zero or non-zero value to set in the direct_io flag
- *
- * Returns Nothing.
- */
-inline void
-mpt3sas_scsi_direct_io_set(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 direct_io)
-{
-	ioc->scsi_lookup[smid - 1].direct_io = direct_io;
-}
-
-/**
  * mpt3sas_setup_direct_io - setup MPI request for WARPDRIVE Direct I/O
  * @ioc: per adapter object
  * @scmd: pointer to scsi command object
@@ -301,12 +287,12 @@
  */
 void
 mpt3sas_setup_direct_io(struct MPT3SAS_ADAPTER *ioc, struct scsi_cmnd *scmd,
-	struct _raid_device *raid_device, Mpi2SCSIIORequest_t *mpi_request,
-	u16 smid)
+	struct _raid_device *raid_device, Mpi2SCSIIORequest_t *mpi_request)
 {
 	sector_t v_lba, p_lba, stripe_off, column, io_size;
 	u32 stripe_sz, stripe_exp;
 	u8 num_pds, cmd = scmd->cmnd[0];
+	struct scsiio_tracker *st = scsi_cmd_priv(scmd);
 
 	if (cmd != READ_10 && cmd != WRITE_10 &&
 	    cmd != READ_16 && cmd != WRITE_16)
@@ -342,5 +328,5 @@
 	else
 		put_unaligned_be64(p_lba, &mpi_request->CDB.CDB32[2]);
 
-	mpt3sas_scsi_direct_io_set(ioc, smid, 1);
+	st->direct_io = 1;
 }
-- 
1.8.5.6

Re: [PATCHv2 09/11] mpt3sas: lockless command submission for scsi-mq

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:08AM +0100, Hannes Reinecke wrote:
> Enable lockless command submission for scsi-mq by moving the
> command structure into the payload for struct request.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c      | 123 ++++-----
>  drivers/scsi/mpt3sas/mpt3sas_base.h      |  13 +-
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  85 ++++--
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 460 ++++++++++++++-----------------
>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  20 +-
>  5 files changed, 330 insertions(+), 371 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index dec86c4..e9470a3 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -865,9 +865,17 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
>  struct scsiio_tracker *
>  mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>  {
> +	u32 unique_tag;
> +	struct scsi_cmnd *cmd;
> +
>  	WARN_ON(!smid);
>  	WARN_ON(smid >= ioc->hi_priority_smid);
> -	return &ioc->scsi_lookup[smid - 1];
> +	unique_tag = smid - 1;
> +	cmd = scsi_host_find_tag(ioc->shost, unique_tag);
> +	if (cmd)
> +		return scsi_cmd_priv(cmd);
> +
> +	return NULL;
>  }
>  
>  /**
> @@ -2343,34 +2351,23 @@ struct scsiio_tracker *
>  mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
>  	struct scsi_cmnd *scmd)
>  {
> -	unsigned long flags;
>  	struct scsiio_tracker *request;
>  	u16 smid;
>  
> -	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
> -	if (list_empty(&ioc->free_list)) {
> -		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
> -		pr_err(MPT3SAS_FMT "%s: smid not available\n",
> -		    ioc->name, __func__);
> -		return 0;
> -	}
> -
>  	if (!scmd) {
> -		/* ioctl command, always use the first slot */
> -		request = ioc->lookup[0];
> -		request->scmd = NULL;
> +		smid = 1;
> +		request = mpt3sas_get_st_from_smid(ioc, smid);

How is this going to work?  mpt3sas_get_st_from_smid fails if we
don't find a block layer tag derived from smid?

>  /**
>   * mpt3sas_base_free_smid - put smid back on free_list
>   * @ioc: per adapter object
> @@ -2428,22 +2441,21 @@ struct scsiio_tracker *
>  	unsigned long flags;
>  	int i;
>  
>  	if (smid < ioc->hi_priority_smid) {
> +		struct scsiio_tracker *st;
>  
> +		st = mpt3sas_get_st_from_smid(ioc, smid);
> +		if (WARN_ON(!st)) {
> +			_base_recovery_check(ioc);
> +			return;
> +		}
> +		mpt3sas_base_clear_st(ioc, st);
>  		_base_recovery_check(ioc);

		st = mpt3sas_get_st_from_smid(ioc, smid);
		if (!WARN_ON_ONCE(!st))
			mpt3sas_base_clear_st(ioc, st);
		_base_recovery_check(ioc);
		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);
> +	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
> +				_count_pending, ioc);

You can't rely on blk-mq being used, and we'd really want to avoid
layering violations like this anyway.

Re: [PATCHv2 09/11] mpt3sas: lockless command submission for scsi-mq

[Hannes Reinecke]

On 02/17/2017 09:54 AM, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 09:23:08AM +0100, Hannes Reinecke wrote:
>> Enable lockless command submission for scsi-mq by moving the
>> command structure into the payload for struct request.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c      | 123 ++++-----
>>  drivers/scsi/mpt3sas/mpt3sas_base.h      |  13 +-
>>  drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  85 ++++--
>>  drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 460 ++++++++++++++-----------------
>>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  20 +-
>>  5 files changed, 330 insertions(+), 371 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index dec86c4..e9470a3 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -865,9 +865,17 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
>>  struct scsiio_tracker *
>>  mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>>  {
>> +	u32 unique_tag;
>> +	struct scsi_cmnd *cmd;
>> +
>>  	WARN_ON(!smid);
>>  	WARN_ON(smid >= ioc->hi_priority_smid);
>> -	return &ioc->scsi_lookup[smid - 1];
>> +	unique_tag = smid - 1;
>> +	cmd = scsi_host_find_tag(ioc->shost, unique_tag);
>> +	if (cmd)
>> +		return scsi_cmd_priv(cmd);
>> +
>> +	return NULL;
>>  }
>>  
>>  /**
>> @@ -2343,34 +2351,23 @@ struct scsiio_tracker *
>>  mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
>>  	struct scsi_cmnd *scmd)
>>  {
>> -	unsigned long flags;
>>  	struct scsiio_tracker *request;
>>  	u16 smid;
>>  
>> -	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
>> -	if (list_empty(&ioc->free_list)) {
>> -		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
>> -		pr_err(MPT3SAS_FMT "%s: smid not available\n",
>> -		    ioc->name, __func__);
>> -		return 0;
>> -	}
>> -
>>  	if (!scmd) {
>> -		/* ioctl command, always use the first slot */
>> -		request = ioc->lookup[0];
>> -		request->scmd = NULL;
>> +		smid = 1;
>> +		request = mpt3sas_get_st_from_smid(ioc, smid);
> 
> How is this going to work?  mpt3sas_get_st_from_smid fails if we
> don't find a block layer tag derived from smid?
> 
Yes, true; it will fail.

It will be fixed up by using reserved commands in patch 11.

>>  	/* 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);
>> +	blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
>> +				_count_pending, ioc);
> 
> You can't rely on blk-mq being used, and we'd really want to avoid
> layering violations like this anyway.
> 
Well, I _did_ enable blk-mq later on, so from that perspective,
yes, I can.

But if that's a layering violation, how am I supposed to check this?
Would be a wrapper in the scsi midlayer be acceptable?
Or is using blk_mq_tagset_busy_iter considered internal to the block layer?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv2 09/11] mpt3sas: lockless command submission for scsi-mq

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 10:03:03AM +0100, Hannes Reinecke wrote:
> Yes, true; it will fail.
> 
> It will be fixed up by using reserved commands in patch 11.

Patches should be fully bisectable.  If that is too much work they
need to be merged with a good explanation on why it is done.

> > You can't rely on blk-mq being used, and we'd really want to avoid
> > layering violations like this anyway.
> > 
> Well, I _did_ enable blk-mq later on, so from that perspective,
> yes, I can.

No you can't.  Drivers can not enable blk-mq, it's a global policy
setting.

> But if that's a layering violation, how am I supposed to check this?
> Would be a wrapper in the scsi midlayer be acceptable?
> Or is using blk_mq_tagset_busy_iter considered internal to the block layer?

I think the right answer is to figure out what you're actually doing,
and if it's a) nessecary at all, b) if it's already implemented in common
code and c) if it should be implemented in common code.

[PATCHv2 10/11] scsi: allocate reserved commands

[Hannes Reinecke]

The block layer already has the notion of 'reserved' commands, so
we should be enabling hosts to allocate them.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_lib.c  | 1 +
 include/scsi/scsi_host.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c35b6de..9d6aed5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2129,6 +2129,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 	shost->tag_set.ops = &scsi_mq_ops;
 	shost->tag_set.nr_hw_queues = shost->nr_hw_queues ? : 1;
 	shost->tag_set.queue_depth = shost->can_queue;
+	shost->tag_set.reserved_tags = shost->reserved_cmds;
 	shost->tag_set.cmd_size = cmd_size;
 	shost->tag_set.numa_node = NUMA_NO_NODE;
 	shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 36680f1..cc83dd6 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -622,6 +622,7 @@ struct Scsi_Host {
 
 	int this_id;
 	int can_queue;
+	int reserved_cmds;
 	short cmd_per_lun;
 	short unsigned int sg_tablesize;
 	short unsigned int sg_prot_tablesize;
-- 
1.8.5.6

Re: [PATCHv2 10/11] scsi: allocate reserved commands

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:09AM +0100, Hannes Reinecke wrote:
> The block layer already has the notion of 'reserved' commands, so
> we should be enabling hosts to allocate them.

How does this interact with the non blk-mq path?

Re: [PATCHv2 10/11] scsi: allocate reserved commands

[Hannes Reinecke]

On 02/17/2017 09:55 AM, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 09:23:09AM +0100, Hannes Reinecke wrote:
>> The block layer already has the notion of 'reserved' commands, so
>> we should be enabling hosts to allocate them.
> 
> How does this interact with the non blk-mq path?
> 
It doesn't; legacy sq doesn't have the notion of 'reserved' command.

So here's my question: how are 'reserved' commands are imagined to be used?
ATM they exist for blk-mq solely, and are not even implemented for
legacy sq.
At the same time quite some drivers (like mpt3sas) really could make use
of the notion of 'reserved' commands.
Does it make sense to implement 'reserved' commands for the legacy sq
code path, too?
I'd be happy to implement something like this; would make life far
easier in certain other places, too (fnic springs to mind ...)

Bug if so, would such a patch being accepted?
Any attempts to modify legacy sq has been met with heavy resistance so
far ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv2 10/11] scsi: allocate reserved commands

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 10:00:28AM +0100, Hannes Reinecke wrote:
> So here's my question: how are 'reserved' commands are imagined to be used?
> ATM they exist for blk-mq solely, and are not even implemented for
> legacy sq.

Yes.

> At the same time quite some drivers (like mpt3sas) really could make use
> of the notion of 'reserved' commands.
> Does it make sense to implement 'reserved' commands for the legacy sq
> code path, too?

Yes.

[PATCHv2 11/11] mpt3sas: register reserved commands

[Hannes Reinecke]

The mpt3sas driver requires a reserved command space to handle
SCSI passthrough commands.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index e9470a3..97189ad 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2360,6 +2360,8 @@ struct scsiio_tracker *
 	} else {
 		u32 unique_tag = blk_mq_unique_tag(scmd->request);
 		u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
+
+		WARN_ON(tag < ioc->shost->reserved_cmds);
 		request = scsi_cmd_priv(scmd);
 		smid = tag + 1;
 	}
@@ -3521,7 +3523,8 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
 	/* set the scsi host can_queue depth
 	 * with some internal commands that could be outstanding
 	 */
-	ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;
+	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;
+	ioc->shost->can_queue = ioc->scsiio_depth - ioc->shost->reserved_cmds;
 	dinitprintk(ioc, pr_info(MPT3SAS_FMT
 		"scsi host: can_queue depth (%d)\n",
 		ioc->name, ioc->shost->can_queue));
-- 
1.8.5.6

Re: [PATCHv2 11/11] mpt3sas: register reserved commands

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 09:23:10AM +0100, Hannes Reinecke wrote:
> The mpt3sas driver requires a reserved command space to handle
> SCSI passthrough commands.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index e9470a3..97189ad 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2360,6 +2360,8 @@ struct scsiio_tracker *
>  	} else {
>  		u32 unique_tag = blk_mq_unique_tag(scmd->request);
>  		u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
> +
> +		WARN_ON(tag < ioc->shost->reserved_cmds);
>  		request = scsi_cmd_priv(scmd);
>  		smid = tag + 1;
>  	}
> @@ -3521,7 +3523,8 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
>  	/* set the scsi host can_queue depth
>  	 * with some internal commands that could be outstanding
>  	 */
> -	ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;
> +	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;
> +	ioc->shost->can_queue = ioc->scsiio_depth - ioc->shost->reserved_cmds;

You're never allocating a reserved request.  Just remove the number of
reserved cmds from can_queue and you can use them on your own.

So I don't think you'll actually need to use reserved request here,
but instead you should set up can_queue properly earlier in the series.

Re: [PATCHv2 11/11] mpt3sas: register reserved commands

[Hannes Reinecke]

On 02/17/2017 01:38 PM, Christoph Hellwig wrote:
> On Fri, Feb 17, 2017 at 09:23:10AM +0100, Hannes Reinecke wrote:
>> The mpt3sas driver requires a reserved command space to handle
>> SCSI passthrough commands.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index e9470a3..97189ad 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -2360,6 +2360,8 @@ struct scsiio_tracker *
>>  	} else {
>>  		u32 unique_tag = blk_mq_unique_tag(scmd->request);
>>  		u16 tag = blk_mq_unique_tag_to_tag(unique_tag);
>> +
>> +		WARN_ON(tag < ioc->shost->reserved_cmds);
>>  		request = scsi_cmd_priv(scmd);
>>  		smid = tag + 1;
>>  	}
>> @@ -3521,7 +3523,8 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
>>  	/* set the scsi host can_queue depth
>>  	 * with some internal commands that could be outstanding
>>  	 */
>> -	ioc->shost->can_queue = ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT;
>> +	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;
>> +	ioc->shost->can_queue = ioc->scsiio_depth - ioc->shost->reserved_cmds;
> 
> You're never allocating a reserved request.  Just remove the number of
> reserved cmds from can_queue and you can use them on your own.
> 
> So I don't think you'll actually need to use reserved request here,
> but instead you should set up can_queue properly earlier in the series.
> 
Well, once I'm switching over to embedded commands I'll have to
integrate with SCSI midlayer to actually _allocate_ the tracker
structure for me.
At the same time the midlayer should _not_ use that structure for normal
I/O; which sounds suspiciously like reserved commands to me ...
Oh well; guess I'll have to do it, then.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv2 11/11] mpt3sas: register reserved commands

[Christoph Hellwig]

On Fri, Feb 17, 2017 at 02:18:42PM +0100, Hannes Reinecke wrote:
> Well, once I'm switching over to embedded commands I'll have to
> integrate with SCSI midlayer to actually _allocate_ the tracker
> structure for me.
> At the same time the midlayer should _not_ use that structure for normal
> I/O; which sounds suspiciously like reserved commands to me ...

Let's defer the issue until that code is actually introduced.  I think
having reserved tag support even for !mq will be useful for the transition
period, so I'm not against it.  But as far as this series is concerned
it's not actually needed, or to be more blunt incorrectly used.