[PATCHv3 00/10] mpt3sas: Full mq support, part 1

[PATCHv3 00/10] 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

Changes to v2:
- Rework ioctl code to not use blk_mq_busy_iter
- Open-code mpt3sas_scsi_direct_io_(get,set)

Hannes Reinecke (10):
  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
  scsi: allocate reserved commands
  mpt3sas: always use first reserved smid for ioctl passthrough
  mpt3sas: lockless command submission for scsi-mq

 drivers/scsi/mpt3sas/mpt3sas_base.c      | 276 +++++++++++++++----------------
 drivers/scsi/mpt3sas/mpt3sas_base.h      |  23 +--
 drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  30 ++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 180 +++++++-------------
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  33 +---
 drivers/scsi/scsi_lib.c                  |   1 +
 include/scsi/scsi_host.h                 |   1 +
 7 files changed, 223 insertions(+), 321 deletions(-)

-- 
1.8.5.6

[PATCHv3 01/10] 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

[PATCHv3 02/10] 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

[PATCHv3 03/10] 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 3062171..6229eda 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2395,20 +2395,13 @@ 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(&ioc->scsi_lookup[i].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

[PATCHv3 04/10] 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 6229eda..0133e7c 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
@@ -2408,15 +2421,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

[PATCHv3 05/10] 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

[PATCHv3 06/10] 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      | 22 ++++++++++++++++++----
 drivers/scsi/mpt3sas/mpt3sas_base.h      |  2 ++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     |  7 ++++---
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  4 +++-
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 0133e7c..3f9148c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -862,6 +862,15 @@ 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)
+{
+	if (WARN_ON(!smid) ||
+	    WARN_ON(smid >= ioc->hi_priority_smid))
+		return NULL;
+	return &ioc->scsi_lookup[smid - 1];
+}
+
 /**
  * _base_get_cb_idx - obtain the callback index
  * @ioc: per adapter object
@@ -876,8 +885,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 +1280,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 +1293,9 @@ 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);
+	if (st)
+		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..6bc9291 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",
@@ -2287,7 +2287,8 @@ struct _sas_node *
 	mpt3sas_scsih_set_tm_flag(ioc, handle);
 	init_completion(&ioc->tm_cmds.done);
 	if ((type == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK) &&
-			(scsi_lookup->msix_io < ioc->reply_queue_count))
+	    scsi_lookup &&
+	    (scsi_lookup->msix_io < ioc->reply_queue_count))
 		msix_task = scsi_lookup->msix_io;
 	else
 		msix_task = 0;
@@ -2328,7 +2329,7 @@ struct _sas_node *
 	switch (type) {
 	case MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK:
 		rc = SUCCESS;
-		if (scsi_lookup->scmd == NULL)
+		if (scsi_lookup && scsi_lookup->scmd == NULL)
 			break;
 		rc = FAILED;
 		break;
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

[PATCHv3 07/10] 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 6bc9291..1c45fb3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2261,6 +2261,14 @@ 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)
+			return FAILED;
+		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 +2276,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

[PATCHv3 08/10] scsi: allocate reserved commands

[Hannes Reinecke]

The block layer already has the notion of 'reserved' commands, so
we should be enabling scsi 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

[PATCHv3 09/10] mpt3sas: always use first reserved smid for ioctl passthrough

[Hannes Reinecke]

ioctl passthrough commands require a SCSIIO smid, but cannot
easily integrate with the block layer. But the driver already
has reserved some SCSIIO smids and we're only ever allowing
one ioctl command at a time we can use the first reserved smid
for ioctl commands.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 11 ++++++++---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c  | 10 ++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 3f9148c..0875e58 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2432,7 +2432,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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
+			list_add(&ioc->scsi_lookup[i].tracker_list,
+				 &ioc->free_list);
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
 		_base_recovery_check(ioc);
@@ -5174,8 +5176,11 @@ struct scsiio_tracker *
 		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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
+			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
+				      &ioc->free_list);
+		else
+			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
 	}
 
 	/* hi-priority queue */
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 02fe1c4..23e0ef1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -719,14 +719,8 @@ enum block_state {
 			goto out;
 		}
 	} else {
-
-		smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL);
-		if (!smid) {
-			pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
-			    ioc->name, __func__);
-			ret = -EAGAIN;
-			goto out;
-		}
+		/* Use first reserved smid for passthrough ioctls */
+		smid = ioc->scsiio_depth - ioc->host->reserved_cmds;
 	}
 
 	ret = 0;
-- 
1.8.5.6

[PATCHv3 10/10] 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      |  19 ++--
 drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  22 +++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 153 ++++++++++---------------------
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  35 +------
 5 files changed, 128 insertions(+), 224 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 0875e58..0177d37 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -865,10 +865,19 @@ 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;
+
 	if (WARN_ON(!smid) ||
 	    WARN_ON(smid >= ioc->hi_priority_smid))
 		return NULL;
-	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;
 }
 
 /**
@@ -2345,26 +2354,22 @@ 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;
+	struct scsiio_tracker *request = scsi_cmd_priv(scmd);
+	unsigned int tag;
 	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 (ioc->shost->use_blk_mq) {
+		u32 unique_tag = blk_mq_unique_tag(scmd->request);
+
+		tag = blk_mq_unique_tag_to_tag(unique_tag);
+	} else
+		tag = scmd->request->tag;
 
-	request = list_entry(ioc->free_list.next,
-	    struct scsiio_tracker, tracker_list);
-	request->scmd = scmd;
+	smid = tag + 1;
 	request->cb_idx = cb_idx;
-	smid = request->smid;
 	request->msix_io = _base_get_msix_index(ioc);
-	list_del(&request->tracker_list);
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+	request->smid = smid;
+	INIT_LIST_HEAD(&request->chain_list);
 	return smid;
 }
 
@@ -2410,6 +2415,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
@@ -2423,23 +2444,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(&ioc->scsi_lookup[i].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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
-			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;
@@ -3272,10 +3291,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) {
@@ -3509,7 +3524,8 @@ struct scsiio_tracker *
 	/* 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));
@@ -3569,16 +3585,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));
 
@@ -5166,22 +5172,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 = 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;
-		if (i < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
-			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
-				      &ioc->free_list);
-		else
-			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
-	}
 
 	/* hi-priority queue */
 	INIT_LIST_HEAD(&ioc->hpr_free_list);
@@ -5692,8 +5683,7 @@ struct scsiio_tracker *
 _wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
 {
 	u32 ioc_state;
-	unsigned long flags;
-	u16 i;
+	u16 smid;
 
 	ioc->pending_io_count = 0;
 
@@ -5702,12 +5692,13 @@ 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);
+	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
+		struct scsiio_tracker *st;
 
+		st = mpt3sas_get_st_from_smid(ioc, smid);
+		if (st->cb_idx != 0xFF)
+			ioc->pending_io_count++;
+	}
 	if (!ioc->pending_io_count)
 		return;
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 74186e3..71b438e 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);
@@ -1302,6 +1298,8 @@ void mpt3sas_device_remove_by_sas_address(struct MPT3SAS_ADAPTER *ioc,
 	u64 sas_address);
 u8 mpt3sas_check_for_pending_internal_cmds(struct MPT3SAS_ADAPTER *ioc,
 	u16 smid);
+struct scsi_cmnd *mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc,
+	u16 smid);
 
 struct _sas_node *mpt3sas_scsih_expander_find_by_handle(
 	struct MPT3SAS_ADAPTER *ioc, u16 handle);
@@ -1453,14 +1451,9 @@ void mpt3sas_trigger_mpi(struct MPT3SAS_ADAPTER *ioc, u16 ioc_status,
 u8 mpt3sas_get_num_volumes(struct MPT3SAS_ADAPTER *ioc);
 void mpt3sas_init_warpdrive_properties(struct MPT3SAS_ADAPTER *ioc,
 	struct _raid_device *raid_device);
-u8
-mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid);
-void
-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 23e0ef1..bcc6a51 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -563,11 +563,10 @@ enum block_state {
 	Mpi2SCSITaskManagementRequest_t *tm_request)
 {
 	u8 found = 0;
-	u16 i;
+	u16 smid;
 	u16 handle;
 	struct scsi_cmnd *scmd;
 	struct MPT3SAS_DEVICE *priv_data;
-	unsigned long flags;
 	Mpi2SCSITaskManagementReply_t *tm_reply;
 	u32 sz;
 	u32 lun;
@@ -583,11 +582,11 @@ enum block_state {
 	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)
+	for (smid = ioc->scsiio_depth; smid && !found; smid--) {
+		struct scsiio_tracker *st;
+
+		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
+		if (!scmd)
 			continue;
 		if (lun != scmd->device->lun)
 			continue;
@@ -596,10 +595,10 @@ enum block_state {
 			continue;
 		if (priv_data->sas_target->handle != handle)
 			continue;
-		tm_request->TaskMID = cpu_to_le16(ioc->scsi_lookup[i - 1].smid);
+		st = scsi_cmd_priv(scmd);
+		tm_request->TaskMID = cpu_to_le16(st->smid);
 		found = 1;
 	}
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
 	if (!found) {
 		dctlprintk(ioc, pr_info(MPT3SAS_FMT
@@ -720,7 +719,10 @@ enum block_state {
 		}
 	} else {
 		/* Use first reserved smid for passthrough ioctls */
-		smid = ioc->scsiio_depth - ioc->host->reserved_cmds;
+		if (ioc->shost->use_blk_mq)
+			smid = 1;
+		else
+			smid = ioc->scsiio_depth - ioc->shost->reserved_cmds;
 	}
 
 	ret = 0;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 1c45fb3..55d062a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1061,75 +1061,30 @@ struct _sas_node *
 }
 
 /**
- * __scsih_scsi_lookup_get_clear - returns scmd entry without
- *						holding any lock.
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Returns the smid stored scmd pointer.
- * 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
+ * mpt3sas_scsih_scsi_lookup_get - 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)
+struct scsi_cmnd *
+mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 {
-	unsigned long flags;
-	struct scsi_cmnd *scmd;
-
-	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;
-}
+	struct scsi_cmnd *scmd = NULL;
+	struct scsiio_tracker *st;
 
-/**
- * _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)
-{
-	u16 smid;
-	unsigned long	flags;
-	int i;
+	if (smid > 0) {
+		u32 unique_tag = smid - 1;
 
-	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;
+		scmd = scsi_host_find_tag(ioc->shost, unique_tag);
+		if (scmd) {
+			st = scsi_cmd_priv(scmd);
+			if (st->cb_idx == 0xFF)
+				scmd = NULL;
 		}
 	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
-	return smid;
+	return scmd;
 }
 
 /**
@@ -1146,22 +1101,21 @@ struct _sas_node *
 _scsih_scsi_lookup_find_by_target(struct MPT3SAS_ADAPTER *ioc, int id,
 	int channel)
 {
-	u8 found;
-	unsigned long	flags;
-	int i;
+	u8 found = 0;
+	u16 smid;
 
-	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)) {
+	for (smid = 1; smid  <= ioc->scsiio_depth; smid++) {
+		struct scsi_cmnd *scmd;
+
+		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
+		if (!scmd)
+			continue;
+		if (scmd->device->id == id &&
+		    scmd->device->channel == channel) {
 			found = 1;
-			goto out;
+			break;
 		}
 	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 	return found;
 }
 
@@ -1180,23 +1134,22 @@ 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;
+	u8 found = 0;
+	u16 smid;
 
-	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)) {
+	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
+		struct scsi_cmnd *scmd;
+
+		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
+		if (!scmd)
+			continue;
+		if (scmd->device->id == id &&
+		    scmd->device->channel == channel &&
+		    scmd->device->lun == lun) {
 			found = 1;
-			goto out;
+			break;
 		}
 	}
- out:
-	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 	return found;
 }
 
@@ -2334,7 +2287,7 @@ struct _sas_node *
 	switch (type) {
 	case MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK:
 		rc = SUCCESS;
-		if (scsi_lookup && scsi_lookup->scmd == NULL)
+		if (scsi_lookup && scsi_lookup->cb_idx == 0xFF)
 			break;
 		rc = FAILED;
 		break;
@@ -2454,6 +2407,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;
+	struct scsiio_tracker *st = scsi_cmd_priv(scmd);
 	u16 smid;
 	u16 handle;
 	int r;
@@ -2473,8 +2427,7 @@ 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) {
+	if (st->cb_idx == 0xFF); {
 		scmd->result = DID_RESET << 16;
 		r = SUCCESS;
 		goto out;
@@ -3930,10 +3883,10 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 {
 	struct scsi_cmnd *scmd;
 	u16 smid;
-	u16 count = 0;
+	int count = 0;
 
 	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
-		scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
+		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
 		if (!scmd)
 			continue;
 		count++;
@@ -4183,8 +4136,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) {
@@ -4648,6 +4600,7 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	Mpi2SCSIIORequest_t *mpi_request;
 	Mpi2SCSIIOReply_t *mpi_reply;
 	struct scsi_cmnd *scmd;
+	struct scsiio_tracker *st;
 	u16 ioc_status;
 	u32 xfer_cnt;
 	u8 scsi_state;
@@ -4655,16 +4608,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 = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
 	if (scmd == NULL)
 		return 1;
 
@@ -4690,13 +4637,11 @@ static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
 	 * WARPDRIVE: If direct_io is set then it is directIO,
 	 * the failed direct I/O should be redirected to volume
 	 */
-	if (mpt3sas_scsi_direct_io_get(ioc, smid) &&
+	st = scsi_cmd_priv(scmd);
+	if (st->direct_io &&
 	     ((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);
+		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);
@@ -6119,7 +6064,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 = ioc->scsi_lookup[smid - 1].scmd;
+		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
 		if (!scmd)
 			continue;
 		sdev = scmd->device;
@@ -8618,6 +8563,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 */
@@ -8656,6 +8602,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 */
diff --git a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
index 06e3f7d..9e5309d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
@@ -261,35 +261,6 @@
 }
 
 /**
- * mpt3sas_scsi_direct_io_get - returns direct io flag
- * @ioc: per adapter object
- * @smid: system request message index
- *
- * Returns the smid stored scmd pointer.
- */
-inline u8
-mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
-{
-	struct scsiio_tracker *st = mpt3sas_get_st_from_smid(ioc, smid);
-
-	return st ? st->direct_io : 0;
-}
-
-/**
- * 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 +272,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 +313,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: [PATCHv3 08/10] scsi: allocate reserved commands

[Christoph Hellwig]

On Tue, Feb 21, 2017 at 01:27:07PM +0100, Hannes Reinecke wrote:
> The block layer already has the notion of 'reserved' commands, so
> we should be enabling scsi hosts to allocate them.

Only if using blk-mq so far, so this patch is incomplete.  But
fortunately I don't think you actually need this patch, see my reply to
the next one for details.

Re: [PATCHv3 09/10] mpt3sas: always use first reserved smid for ioctl passthrough

[Christoph Hellwig]

On Tue, Feb 21, 2017 at 01:27:08PM +0100, Hannes Reinecke wrote:
> ioctl passthrough commands require a SCSIIO smid, but cannot
> easily integrate with the block layer. But the driver already
> has reserved some SCSIIO smids and we're only ever allowing
> one ioctl command at a time we can use the first reserved smid
> for ioctl commands.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 11 ++++++++---
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c  | 10 ++--------
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 3f9148c..0875e58 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2432,7 +2432,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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
> +			list_add(&ioc->scsi_lookup[i].tracker_list,
> +				 &ioc->free_list);
>  		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
>  
>  		_base_recovery_check(ioc);
> @@ -5174,8 +5176,11 @@ struct scsiio_tracker *
>  		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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
> +			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
> +				      &ioc->free_list);
> +		else
> +			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);

Why the INIT_LIST_HEAD?  We never do a list_empty check on tracker_list,
so it's rather pointless.

> +		smid = ioc->scsiio_depth - ioc->host->reserved_cmds;

ioc->host->reserved_cmds is never set at this point.  But given that only
the smids < ioc->scsiio_depth are used it's doing the right thing
if you just remove the "- ioc->host->reserved_cmds" entirely.

Otherwise this looks fine to me.

Re: [PATCHv3 10/10] mpt3sas: lockless command submission for scsi-mq

[Christoph Hellwig]

On Tue, Feb 21, 2017 at 01:27:09PM +0100, Hannes Reinecke wrote:
> Enable lockless command submission for scsi-mq by moving the
> command structure into the payload for struct request.

No dependency on scsi-mq, so the changelog could use a little update.

> @@ -2345,26 +2354,22 @@ struct scsiio_tracker *
>  mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
>  	struct scsi_cmnd *scmd)
>  {
> +	struct scsiio_tracker *request = scsi_cmd_priv(scmd);
> +	unsigned int tag;
>  	u16 smid;
>  
> +	if (ioc->shost->use_blk_mq) {
> +		u32 unique_tag = blk_mq_unique_tag(scmd->request);
> +
> +		tag = blk_mq_unique_tag_to_tag(unique_tag);
> +	} else
> +		tag = scmd->request->tag;

All that unique_tag stuff is only required for SCSI LLDDs that
set nr_hw_queues > 1, which isn't the ase with this patch.

>   * @ioc: per adapter object
> @@ -2423,23 +2444,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);
>  		return;

This looks fine, but it would be good if could derive the scsiio_tracker
structure from the scsi_cmnd in as many as possible places, most notably
all the fast path calls and then just clal mpt3sas_base_clear_st directly.

> +	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;

Why?  You're never calling the block layer to allocate a reserved
request.

> +	ioc->shost->can_queue = ioc->scsiio_depth - ioc->shost->reserved_cmds;

Just assign ioc->shost->can_queue to ioc->scsiio_depth -
INTERNAL_SCSIIO_CMDS_COUNT directly.

> +	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
> +		struct scsiio_tracker *st;
>  
> +		st = mpt3sas_get_st_from_smid(ioc, smid);
> +		if (st->cb_idx != 0xFF)
> +			ioc->pending_io_count++;
> +	}

How is this different from ioc->host->host_busy (except for maybe
the ioctl smid).

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> index 23e0ef1..bcc6a51 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> @@ -563,11 +563,10 @@ enum block_state {
>  	Mpi2SCSITaskManagementRequest_t *tm_request)
>  {
>  	u8 found = 0;
> -	u16 i;
> +	u16 smid;
>  	u16 handle;
>  	struct scsi_cmnd *scmd;
>  	struct MPT3SAS_DEVICE *priv_data;
> -	unsigned long flags;
>  	Mpi2SCSITaskManagementReply_t *tm_reply;
>  	u32 sz;
>  	u32 lun;
> @@ -583,11 +582,11 @@ enum block_state {
>  	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)
> +	for (smid = ioc->scsiio_depth; smid && !found; smid--) {
> +		struct scsiio_tracker *st;
> +
> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> +		if (!scmd)
>  			continue;
>  		if (lun != scmd->device->lun)
>  			continue;
> @@ -596,10 +595,10 @@ enum block_state {
>  			continue;
>  		if (priv_data->sas_target->handle != handle)
>  			continue;
> -		tm_request->TaskMID = cpu_to_le16(ioc->scsi_lookup[i - 1].smid);
> +		st = scsi_cmd_priv(scmd);
> +		tm_request->TaskMID = cpu_to_le16(st->smid);
>  		found = 1;
>  	}
> -	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);

This looks like you're keeping the existing behavior, but can someone
please explain why we need to find a random used smid here?  Whatever
requests is using the smid could be completed at any point in time,
so this whole loop looks rather bogus both in the old and new version.
a

> +		if (ioc->shost->use_blk_mq)
> +			smid = 1;
> +		else
> +			smid = ioc->scsiio_depth - ioc->shost->reserved_cmds;

Huh?  Explanation, please.

> @@ -1146,22 +1101,21 @@ struct _sas_node *
>  _scsih_scsi_lookup_find_by_target(struct MPT3SAS_ADAPTER *ioc, int id,
>  	int channel)
>  {
> +	u8 found = 0;
> +	u16 smid;
>  
> +	for (smid = 1; smid  <= ioc->scsiio_depth; smid++) {
> +		struct scsi_cmnd *scmd;
> +
> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> +		if (!scmd)
> +			continue;
> +		if (scmd->device->id == id &&
> +		    scmd->device->channel == channel) {
>  			found = 1;
> +			break;
>  		}
>  	}
>  	return found;
>  }

This looks ok, but I'd really like to understand what the hell
the original code is trying to do here.

>  
> @@ -1180,23 +1134,22 @@ struct _sas_node *
>  _scsih_scsi_lookup_find_by_lun(struct MPT3SAS_ADAPTER *ioc, int id,
>  	unsigned int lun, int channel)
>  {
> +	u8 found = 0;
> +	u16 smid;
>  
> +	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
> +		struct scsi_cmnd *scmd;
> +
> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> +		if (!scmd)
> +			continue;
> +		if (scmd->device->id == id &&
> +		    scmd->device->channel == channel &&
> +		    scmd->device->lun == lun) {
>  			found = 1;
> +			break;
>  		}
>  	}
>  	return found;
>  }

.. and here

Re: [PATCHv3 09/10] mpt3sas: always use first reserved smid for ioctl passthrough

[Hannes Reinecke]

On 02/21/2017 03:18 PM, Christoph Hellwig wrote:
> On Tue, Feb 21, 2017 at 01:27:08PM +0100, Hannes Reinecke wrote:
>> ioctl passthrough commands require a SCSIIO smid, but cannot
>> easily integrate with the block layer. But the driver already
>> has reserved some SCSIIO smids and we're only ever allowing
>> one ioctl command at a time we can use the first reserved smid
>> for ioctl commands.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 11 ++++++++---
>>  drivers/scsi/mpt3sas/mpt3sas_ctl.c  | 10 ++--------
>>  2 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 3f9148c..0875e58 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -2432,7 +2432,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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
>> +			list_add(&ioc->scsi_lookup[i].tracker_list,
>> +				 &ioc->free_list);
>>  		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
>>  
>>  		_base_recovery_check(ioc);
>> @@ -5174,8 +5176,11 @@ struct scsiio_tracker *
>>  		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 < ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT)
>> +			list_add_tail(&ioc->scsi_lookup[i].tracker_list,
>> +				      &ioc->free_list);
>> +		else
>> +			INIT_LIST_HEAD(&ioc->lookup[i].tracker_list);
> 
> Why the INIT_LIST_HEAD?  We never do a list_empty check on tracker_list,
> so it's rather pointless.
> 
Okay.

>> +		smid = ioc->scsiio_depth - ioc->host->reserved_cmds;
> 
> ioc->host->reserved_cmds is never set at this point.  But given that only
> the smids < ioc->scsiio_depth are used it's doing the right thing
> if you just remove the "- ioc->host->reserved_cmds" entirely.
> 
Not quite; then it'll clash with the hi-priority smids which are
allocated after that.
But I can be using ioc->scsio_depth - INTERNAL_SCSIIO_CMDS_COUNT 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)

Re: [PATCHv3 10/10] mpt3sas: lockless command submission for scsi-mq

[Hannes Reinecke]

On 02/21/2017 03:34 PM, Christoph Hellwig wrote:
> On Tue, Feb 21, 2017 at 01:27:09PM +0100, Hannes Reinecke wrote:
>> Enable lockless command submission for scsi-mq by moving the
>> command structure into the payload for struct request.
> 
> No dependency on scsi-mq, so the changelog could use a little update.
> 
>> @@ -2345,26 +2354,22 @@ struct scsiio_tracker *
>>  mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
>>  	struct scsi_cmnd *scmd)
>>  {
>> +	struct scsiio_tracker *request = scsi_cmd_priv(scmd);
>> +	unsigned int tag;
>>  	u16 smid;
>>  
>> +	if (ioc->shost->use_blk_mq) {
>> +		u32 unique_tag = blk_mq_unique_tag(scmd->request);
>> +
>> +		tag = blk_mq_unique_tag_to_tag(unique_tag);
>> +	} else
>> +		tag = scmd->request->tag;
> 
> All that unique_tag stuff is only required for SCSI LLDDs that
> set nr_hw_queues > 1, which isn't the ase with this patch.
> 
Abstraction.
I would not presume anything about SCSI-MQ/block-mq here; calling
blk_mq_unique_tag() insulates me against any changes blk-mq might be
doing for the tag allocation or management.

>>   * @ioc: per adapter object
>> @@ -2423,23 +2444,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);
>>  		return;
> 
> This looks fine, but it would be good if could derive the scsiio_tracker
> structure from the scsi_cmnd in as many as possible places, most notably
> all the fast path calls and then just clal mpt3sas_base_clear_st directly.
> 
Okay, will be revisiting that.

>> +	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;
> 
> Why?  You're never calling the block layer to allocate a reserved
> request.
> 
The idea was to have a stub command for any ioctl passthrough commands,
so they'll show up when traversing the list of active commands (hence it
always uses 'scsiio_depth' and not 'can_queue').
But it might be that we're not setting the scsiio_tracker for ioctl
passthrough commands; will be checking.

>> +	ioc->shost->can_queue = ioc->scsiio_depth - ioc->shost->reserved_cmds;
> 
> Just assign ioc->shost->can_queue to ioc->scsiio_depth -
> INTERNAL_SCSIIO_CMDS_COUNT directly.
> 
Okay.

>> +	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
>> +		struct scsiio_tracker *st;
>>  
>> +		st = mpt3sas_get_st_from_smid(ioc, smid);
>> +		if (st->cb_idx != 0xFF)
>> +			ioc->pending_io_count++;
>> +	}
> 
> How is this different from ioc->host->host_busy (except for maybe
> the ioctl smid).
> 
This is just a plain conversion from the existing code; but yes, one
could be using host_busy here.

>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> index 23e0ef1..bcc6a51 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> @@ -563,11 +563,10 @@ enum block_state {
>>  	Mpi2SCSITaskManagementRequest_t *tm_request)
>>  {
>>  	u8 found = 0;
>> -	u16 i;
>> +	u16 smid;
>>  	u16 handle;
>>  	struct scsi_cmnd *scmd;
>>  	struct MPT3SAS_DEVICE *priv_data;
>> -	unsigned long flags;
>>  	Mpi2SCSITaskManagementReply_t *tm_reply;
>>  	u32 sz;
>>  	u32 lun;
>> @@ -583,11 +582,11 @@ enum block_state {
>>  	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)
>> +	for (smid = ioc->scsiio_depth; smid && !found; smid--) {
>> +		struct scsiio_tracker *st;
>> +
>> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
>> +		if (!scmd)
>>  			continue;
>>  		if (lun != scmd->device->lun)
>>  			continue;
>> @@ -596,10 +595,10 @@ enum block_state {
>>  			continue;
>>  		if (priv_data->sas_target->handle != handle)
>>  			continue;
>> -		tm_request->TaskMID = cpu_to_le16(ioc->scsi_lookup[i - 1].smid);
>> +		st = scsi_cmd_priv(scmd);
>> +		tm_request->TaskMID = cpu_to_le16(st->smid);
>>  		found = 1;
>>  	}
>> -	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
> 
> This looks like you're keeping the existing behavior, but can someone
> please explain why we need to find a random used smid here?  Whatever
> requests is using the smid could be completed at any point in time,
> so this whole loop looks rather bogus both in the old and new version.
> a
> 
And so it is.
The whole concept of ABORT TASK ioctl is patently bogus; the caller has
_no_ idea about the tag to abort, so it just fetches the first busy command.

I surely think that this should go, but I have no idea what any
management tools might be doing, so I've kept existing behaviour.

>> +		if (ioc->shost->use_blk_mq)
>> +			smid = 1;
>> +		else
>> +			smid = ioc->scsiio_depth - ioc->shost->reserved_cmds;
> 
> Huh?  Explanation, please.
> 
When enabling reserved tags with blk-mq the reserved tag range is at the
start of the tagspace; for legacy sq there is no reserved range, but
'can_queue' is reduced by the number of reserved commands, effectively
locating the reserved tag range at the end of the tag space.

>> @@ -1146,22 +1101,21 @@ struct _sas_node *
>>  _scsih_scsi_lookup_find_by_target(struct MPT3SAS_ADAPTER *ioc, int id,
>>  	int channel)
>>  {
>> +	u8 found = 0;
>> +	u16 smid;
>>  
>> +	for (smid = 1; smid  <= ioc->scsiio_depth; smid++) {
>> +		struct scsi_cmnd *scmd;
>> +
>> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
>> +		if (!scmd)
>> +			continue;
>> +		if (scmd->device->id == id &&
>> +		    scmd->device->channel == channel) {
>>  			found = 1;
>> +			break;
>>  		}
>>  	}
>>  	return found;
>>  }
> 
> This looks ok, but I'd really like to understand what the hell
> the original code is trying to do here.
> 
Figuring out if there are still pending commands on the given target.

>>  
>> @@ -1180,23 +1134,22 @@ struct _sas_node *
>>  _scsih_scsi_lookup_find_by_lun(struct MPT3SAS_ADAPTER *ioc, int id,
>>  	unsigned int lun, int channel)
>>  {
>> +	u8 found = 0;
>> +	u16 smid;
>>  
>> +	for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
>> +		struct scsi_cmnd *scmd;
>> +
>> +		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
>> +		if (!scmd)
>> +			continue;
>> +		if (scmd->device->id == id &&
>> +		    scmd->device->channel == channel &&
>> +		    scmd->device->lun == lun) {
>>  			found = 1;
>> +			break;
>>  		}
>>  	}
>>  	return found;
>>  }
> 
> .. and here
> 
Figuring out if there are pending commands on that LUN.

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: [PATCHv3 10/10] mpt3sas: lockless command submission for scsi-mq

[Christoph Hellwig]

On Tue, Feb 21, 2017 at 03:58:08PM +0100, Hannes Reinecke wrote:
> >> +	if (ioc->shost->use_blk_mq) {
> >> +		u32 unique_tag = blk_mq_unique_tag(scmd->request);
> >> +
> >> +		tag = blk_mq_unique_tag_to_tag(unique_tag);
> >> +	} else
> >> +		tag = scmd->request->tag;
> > 
> > All that unique_tag stuff is only required for SCSI LLDDs that
> > set nr_hw_queues > 1, which isn't the ase with this patch.
> > 
> Abstraction.
> I would not presume anything about SCSI-MQ/block-mq here; calling
> blk_mq_unique_tag() insulates me against any changes blk-mq might be
> doing for the tag allocation or management.

cmd->request->tag has the same meaning with blk-mq or not, so this
check is obviously wrong.

> >> +	ioc->shost->reserved_cmds = INTERNAL_SCSIIO_CMDS_COUNT;
> > 
> > Why?  You're never calling the block layer to allocate a reserved
> > request.
> > 
> The idea was to have a stub command for any ioctl passthrough commands,
> so they'll show up when traversing the list of active commands (hence it
> always uses 'scsiio_depth' and not 'can_queue').

Sure.  But setting reserved_cmds has nothing to do with that - it only
sets requests aside so they can be allocated using BLK_MQ_REQ_RESERVED.

If you reserve tags for purely driver internal usage without a struct
request just don't advertise them to the block layer at all.  This
is what we do in hpsa, and that's what mpt3sas should do as well.

> >> +		if (ioc->shost->use_blk_mq)
> >> +			smid = 1;
> >> +		else
> >> +			smid = ioc->scsiio_depth - ioc->shost->reserved_cmds;
> > 
> > Huh?  Explanation, please.
> > 
> When enabling reserved tags with blk-mq the reserved tag range is at the
> start of the tagspace; for legacy sq there is no reserved range, but
> 'can_queue' is reduced by the number of reserved commands, effectively
> locating the reserved tag range at the end of the tag space.

internal tag number are a blk-mq implementation detail, and drivers
should not second guess it.  As said above - just don't use the
reserved_cmds field and set the smids aside inside mpt3sas.

> Figuring out if there are still pending commands on the given target.

starget->target_busy

> Figuring out if there are pending commands on that LUN.

sdev->device_busy

Re: [PATCHv3 07/10] mpt3sas: check command status before attempting abort

[Sreekanth Reddy]

On Tue, Feb 21, 2017 at 5:57 PM, Hannes Reinecke <hare@suse.de> wrote:
> When attempting a command abort we should check the command status
> prior to sending the abort; the command might've been completed
> already.

I think we are already taking care of this. Before calling
'mpt3sas_scsih_issue_tm()' function in 'scsih_abort()' we are already
verifying whether the scmd exists in the driver's scsi_lookup table or
not. if exits then we are issuing the task abort TM to Firmware
otherwise we will return success status from scsih_abort() without
issuing task abort TM to Firmware as shown below,

smid = _scsih_scsi_lookup_find_by_scmd(ioc, scmd);
        if (!smid) {
                scmd->result = DID_RESET << 16;
                r = SUCCESS;
                goto out;
        }

Thanks,
Sreekanth

>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 6bc9291..1c45fb3 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -2261,6 +2261,14 @@ 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)
> +                       return FAILED;
> +               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 +2276,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: [PATCHv3 01/10] mpt3sas: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

Martin,

can we get at least this patch still in for 4.11?  I'd really like
to see as many intance of the old MSI-X allocation and IRQ affinity
mess dead as soon as possible.

Re: [PATCHv3 01/10] mpt3sas: switch to pci_alloc_irq_vectors

[Hannes Reinecke]

On 02/22/2017 09:28 AM, Christoph Hellwig wrote:
> Martin,
> 
> can we get at least this patch still in for 4.11?  I'd really like
> to see as many intance of the old MSI-X allocation and IRQ affinity
> mess dead as soon as possible.
> 
I'm currently reworking the patchset with the suggestions from
Christoph, will be posting the next iteration later today.

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: [PATCHv3 01/10] mpt3sas: switch to pci_alloc_irq_vectors

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> Martin, can we get at least this patch still in for 4.11?
Christoph> I'd really like to see as many intance of the old MSI-X
Christoph> allocation and IRQ affinity mess dead as soon as possible.

Me too. I'll queue it up for 4.11 unless Broadcom objects. And then
we'll do the rest for 4.12.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCHv3 01/10] mpt3sas: switch to pci_alloc_irq_vectors

[Sreekanth Reddy]

On Thu, Feb 23, 2017 at 6:22 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
>
> Christoph> Martin, can we get at least this patch still in for 4.11?
> Christoph> I'd really like to see as many intance of the old MSI-X
> Christoph> allocation and IRQ affinity mess dead as soon as possible.
>
> Me too. I'll queue it up for 4.11 unless Broadcom objects. And then
> we'll do the rest for 4.12.

This patch looks good. Please consider this patch for 4.11.

We need some time to review remaining patches as it has lot code
refinement and we need to cover some basic test cases.

Thanks,
Sreekanth

>
> --
> Martin K. Petersen      Oracle Linux Engineering