expose queue the queue mapping for SCSI drivers V2

expose queue the queue mapping for SCSI drivers V2

[Christoph Hellwig]

In 4.9 I've added support in the interrupt layer to automatically
assign the interrupt affinity at interrupt allocation time, and
expose that information to blk-mq.

This series extents that so that SCSI driver can pass on the information
as well.  The SCSI part is fairly trivial, although we need to also
export the default queue mapping function in blk-mq to keep things simple.

I've also converted over the smartpqi driver as an example as it's the
easiest of the multiqueue SCSI drivers to convert.

Changes since V1:
 - move the EXPORT_SYMBOL of blk_mq_map_queues to the right patch
 - added Reviewed-by, Acked-by and Tested-by tags

[PATCH 1/3] blk-mq: export blk_mq_map_queues

[Christoph Hellwig]

This will allow SCSI to have a single blk_mq_ops structure that either
lets the LLDD map the queues to PCIe MSIx vectors or use the default.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-mq-cpumap.c  | 1 +
 block/blk-mq.h         | 1 -
 include/linux/blk-mq.h | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 19b1d9c..8e61e86 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -87,6 +87,7 @@ int blk_mq_map_queues(struct blk_mq_tag_set *set)
 	free_cpumask_var(cpus);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(blk_mq_map_queues);
 
 /*
  * We have no quick way of doing reverse lookups. This is only used at
diff --git a/block/blk-mq.h b/block/blk-mq.h
index e5d2524..5347f01 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -38,7 +38,6 @@ void blk_mq_disable_hotplug(void);
 /*
  * CPU -> queue mappings
  */
-int blk_mq_map_queues(struct blk_mq_tag_set *set);
 extern int blk_mq_hw_queue_to_node(unsigned int *map, unsigned int);
 
 static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 535ab2e..6c0fb25 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -237,6 +237,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q);
 void blk_mq_freeze_queue_start(struct request_queue *q);
 int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
 
+int blk_mq_map_queues(struct blk_mq_tag_set *set);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
 
 /*
-- 
2.1.4

Re: [PATCH 1/3] blk-mq: export blk_mq_map_queues

[Martin K. Petersen]

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

Christoph> This will allow SCSI to have a single blk_mq_ops structure
Christoph> that either lets the LLDD map the queues to PCIe MSIx vectors
Christoph> or use the default.

Jens, any objection to me funneling this change through the SCSI tree?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/3] blk-mq: export blk_mq_map_queues

[Jens Axboe]

On Tue, Nov 01 2016, Martin K. Petersen wrote:
> >>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:
> 
> Christoph> This will allow SCSI to have a single blk_mq_ops structure
> Christoph> that either lets the LLDD map the queues to PCIe MSIx vectors
> Christoph> or use the default.
> 
> Jens, any objection to me funneling this change through the SCSI tree?

No, that's fine, you can add my reviewed-by.

-- 
Jens Axboe

Re: [PATCH 1/3] blk-mq: export blk_mq_map_queues

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 2/3] scsi: allow LLDDs to expose the queue mapping to blk-mq

[Christoph Hellwig]

Just hand through the blk-mq map_queues method in the host template.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_lib.c  | 10 ++++++++++
 include/scsi/scsi_host.h |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 2cca9cf..f23ec24 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1990,6 +1990,15 @@ static void scsi_exit_request(void *data, struct request *rq,
 	kfree(cmd->sense_buffer);
 }
 
+static int scsi_map_queues(struct blk_mq_tag_set *set)
+{
+	struct Scsi_Host *shost = container_of(set, struct Scsi_Host, tag_set);
+
+	if (shost->hostt->map_queues)
+		return shost->hostt->map_queues(shost);
+	return blk_mq_map_queues(set);
+}
+
 static u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
 	struct device *host_dev;
@@ -2082,6 +2091,7 @@ static struct blk_mq_ops scsi_mq_ops = {
 	.timeout	= scsi_timeout,
 	.init_request	= scsi_init_request,
 	.exit_request	= scsi_exit_request,
+	.map_queues	= scsi_map_queues,
 };
 
 struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 7e4cd53..36680f1 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -278,6 +278,14 @@ struct scsi_host_template {
 	int (* change_queue_depth)(struct scsi_device *, int);
 
 	/*
+	 * This functions lets the driver expose the queue mapping
+	 * to the block layer.
+	 *
+	 * Status: OPTIONAL
+	 */
+	int (* map_queues)(struct Scsi_Host *shost);
+
+	/*
 	 * This function determines the BIOS parameters for a given
 	 * harddisk.  These tend to be numbers that are made up by
 	 * the host adapter.  Parameters:
-- 
2.1.4

Re: [PATCH 2/3] scsi: allow LLDDs to expose the queue mapping to blk-mq

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 3/3] smartpqi: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

Which cleans up a lot of the MSI-X handling, and allows us to use the
PCI IRQ layer provided vector mapping, which we can then expose to blk-mq.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Acked-by: Don Brace <don.brace@microsemi.com>
Tested-by: Don Brace <don.brace@microsemi.com>
---
 drivers/scsi/smartpqi/smartpqi.h      |   2 -
 drivers/scsi/smartpqi/smartpqi_init.c | 102 +++++++++++-----------------------
 2 files changed, 32 insertions(+), 72 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index 07b6444..b673825 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -929,8 +929,6 @@ struct pqi_ctrl_info {
 	int		max_msix_vectors;
 	int		num_msix_vectors_enabled;
 	int		num_msix_vectors_initialized;
-	u32		msix_vectors[PQI_MAX_MSIX_VECTORS];
-	void		*intr_data[PQI_MAX_MSIX_VECTORS];
 	int		event_irq;
 	struct Scsi_Host *scsi_host;
 
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index a535b26..8702d9c 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -25,6 +25,7 @@
 #include <linux/rtc.h>
 #include <linux/bcd.h>
 #include <linux/cciss_ioctl.h>
+#include <linux/blk-mq-pci.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_device.h>
@@ -2887,19 +2888,19 @@ static irqreturn_t pqi_irq_handler(int irq, void *data)
 
 static int pqi_request_irqs(struct pqi_ctrl_info *ctrl_info)
 {
+	struct pci_dev *pdev = ctrl_info->pci_dev;
 	int i;
 	int rc;
 
-	ctrl_info->event_irq = ctrl_info->msix_vectors[0];
+	ctrl_info->event_irq = pci_irq_vector(pdev, 0);
 
 	for (i = 0; i < ctrl_info->num_msix_vectors_enabled; i++) {
-		rc = request_irq(ctrl_info->msix_vectors[i],
-			pqi_irq_handler, 0,
-			DRIVER_NAME_SHORT, ctrl_info->intr_data[i]);
+		rc = request_irq(pci_irq_vector(pdev, i), pqi_irq_handler, 0,
+			DRIVER_NAME_SHORT, &ctrl_info->queue_groups[i]);
 		if (rc) {
-			dev_err(&ctrl_info->pci_dev->dev,
+			dev_err(&pdev->dev,
 				"irq %u init failed with error %d\n",
-				ctrl_info->msix_vectors[i], rc);
+				pci_irq_vector(pdev, i), rc);
 			return rc;
 		}
 		ctrl_info->num_msix_vectors_initialized++;
@@ -2908,72 +2909,23 @@ static int pqi_request_irqs(struct pqi_ctrl_info *ctrl_info)
 	return 0;
 }
 
-static void pqi_free_irqs(struct pqi_ctrl_info *ctrl_info)
-{
-	int i;
-
-	for (i = 0; i < ctrl_info->num_msix_vectors_initialized; i++)
-		free_irq(ctrl_info->msix_vectors[i],
-			ctrl_info->intr_data[i]);
-}
-
 static int pqi_enable_msix_interrupts(struct pqi_ctrl_info *ctrl_info)
 {
-	unsigned int i;
-	int max_vectors;
-	int num_vectors_enabled;
-	struct msix_entry msix_entries[PQI_MAX_MSIX_VECTORS];
-
-	max_vectors = ctrl_info->num_queue_groups;
-
-	for (i = 0; i < max_vectors; i++)
-		msix_entries[i].entry = i;
-
-	num_vectors_enabled = pci_enable_msix_range(ctrl_info->pci_dev,
-		msix_entries, PQI_MIN_MSIX_VECTORS, max_vectors);
+	int ret;
 
-	if (num_vectors_enabled < 0) {
+	ret = pci_alloc_irq_vectors(ctrl_info->pci_dev,
+			PQI_MIN_MSIX_VECTORS, ctrl_info->num_queue_groups,
+			PCI_IRQ_MSIX | PCI_IRQ_AFFINITY);
+	if (ret < 0) {
 		dev_err(&ctrl_info->pci_dev->dev,
-			"MSI-X init failed with error %d\n",
-			num_vectors_enabled);
-		return num_vectors_enabled;
-	}
-
-	ctrl_info->num_msix_vectors_enabled = num_vectors_enabled;
-	for (i = 0; i < num_vectors_enabled; i++) {
-		ctrl_info->msix_vectors[i] = msix_entries[i].vector;
-		ctrl_info->intr_data[i] = &ctrl_info->queue_groups[i];
+			"MSI-X init failed with error %d\n", ret);
+		return ret;
 	}
 
+	ctrl_info->num_msix_vectors_enabled = ret;
 	return 0;
 }
 
-static void pqi_irq_set_affinity_hint(struct pqi_ctrl_info *ctrl_info)
-{
-	int i;
-	int rc;
-	int cpu;
-
-	cpu = cpumask_first(cpu_online_mask);
-	for (i = 0; i < ctrl_info->num_msix_vectors_initialized; i++) {
-		rc = irq_set_affinity_hint(ctrl_info->msix_vectors[i],
-			get_cpu_mask(cpu));
-		if (rc)
-			dev_err(&ctrl_info->pci_dev->dev,
-				"error %d setting affinity hint for irq vector %u\n",
-				rc, ctrl_info->msix_vectors[i]);
-		cpu = cpumask_next(cpu, cpu_online_mask);
-	}
-}
-
-static void pqi_irq_unset_affinity_hint(struct pqi_ctrl_info *ctrl_info)
-{
-	int i;
-
-	for (i = 0; i < ctrl_info->num_msix_vectors_initialized; i++)
-		irq_set_affinity_hint(ctrl_info->msix_vectors[i], NULL);
-}
-
 static int pqi_alloc_operational_queues(struct pqi_ctrl_info *ctrl_info)
 {
 	unsigned int i;
@@ -4743,6 +4695,13 @@ static int pqi_slave_configure(struct scsi_device *sdev)
 	return 0;
 }
 
+static int pqi_map_queues(struct Scsi_Host *shost)
+{
+	struct pqi_ctrl_info *ctrl_info = shost_to_hba(shost);
+
+	return blk_mq_pci_map_queues(&shost->tag_set, ctrl_info->pci_dev);
+}
+
 static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,
 	void __user *arg)
 {
@@ -5130,6 +5089,7 @@ static struct scsi_host_template pqi_driver_template = {
 	.ioctl = pqi_ioctl,
 	.slave_alloc = pqi_slave_alloc,
 	.slave_configure = pqi_slave_configure,
+	.map_queues = pqi_map_queues,
 	.sdev_attrs = pqi_sdev_attrs,
 	.shost_attrs = pqi_shost_attrs,
 };
@@ -5159,7 +5119,7 @@ static int pqi_register_scsi(struct pqi_ctrl_info *ctrl_info)
 	shost->cmd_per_lun = shost->can_queue;
 	shost->sg_tablesize = ctrl_info->sg_tablesize;
 	shost->transportt = pqi_sas_transport_template;
-	shost->irq = ctrl_info->msix_vectors[0];
+	shost->irq = pci_irq_vector(ctrl_info->pci_dev, 0);
 	shost->unique_id = shost->irq;
 	shost->nr_hw_queues = ctrl_info->num_queue_groups;
 	shost->hostdata[0] = (unsigned long)ctrl_info;
@@ -5409,8 +5369,6 @@ static int pqi_ctrl_init(struct pqi_ctrl_info *ctrl_info)
 	if (rc)
 		return rc;
 
-	pqi_irq_set_affinity_hint(ctrl_info);
-
 	rc = pqi_create_queues(ctrl_info);
 	if (rc)
 		return rc;
@@ -5557,10 +5515,14 @@ static inline void pqi_free_ctrl_info(struct pqi_ctrl_info *ctrl_info)
 
 static void pqi_free_interrupts(struct pqi_ctrl_info *ctrl_info)
 {
-	pqi_irq_unset_affinity_hint(ctrl_info);
-	pqi_free_irqs(ctrl_info);
-	if (ctrl_info->num_msix_vectors_enabled)
-		pci_disable_msix(ctrl_info->pci_dev);
+	int i;
+
+	for (i = 0; i < ctrl_info->num_msix_vectors_initialized; i++) {
+		free_irq(pci_irq_vector(ctrl_info->pci_dev, i),
+				&ctrl_info->queue_groups[i]);
+	}
+
+	pci_free_irq_vectors(ctrl_info->pci_dev);
 }
 
 static void pqi_free_ctrl_resources(struct pqi_ctrl_info *ctrl_info)
-- 
2.1.4

Re: expose queue the queue mapping for SCSI drivers V2

[Martin K. Petersen]

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

Christoph> In 4.9 I've added support in the interrupt layer to
Christoph> automatically assign the interrupt affinity at interrupt
Christoph> allocation time, and expose that information to blk-mq.

Christoph> This series extents that so that SCSI driver can pass on the
Christoph> information as well.  The SCSI part is fairly trivial,
Christoph> although we need to also export the default queue mapping
Christoph> function in blk-mq to keep things simple.

Christoph> I've also converted over the smartpqi driver as an example as
Christoph> it's the easiest of the multiqueue SCSI drivers to convert.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

expose queue the queue mapping for SCSI drivers

[Christoph Hellwig]

In 4.9 I've added support in the interrupt layer to automatically
assign the interrupt affinity at interrupt allocation time, and
expose that information to blk-mq.

This series extents that so that SCSI driver can pass on the information
as well.  The SCSI part is fairly trivial, although we need to also
export the default queue mapping function in blk-mq to keep things simple.

I've also converted over the smartpqi driver as an example as it's the
easiest of the multiqueue SCSI drivers to convert.  I don't actually
have smartpqi hardware, so that part will need additionaly testing.

(I've tested it with virtio_scsi, but that requires a lot of virtio
 changes not ready to post yet)

[PATCH 1/3] blk-mq: export blk_mq_map_queues

[Christoph Hellwig]

This will allow SCSI to have a single blk_mq_ops structure that either
lets the LLDD map the queues to PCIe MSIx vectors or use the default.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.h         | 1 -
 include/linux/blk-mq.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.h b/block/blk-mq.h
index e5d2524..5347f01 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -38,7 +38,6 @@ void blk_mq_disable_hotplug(void);
 /*
  * CPU -> queue mappings
  */
-int blk_mq_map_queues(struct blk_mq_tag_set *set);
 extern int blk_mq_hw_queue_to_node(unsigned int *map, unsigned int);
 
 static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 535ab2e..6c0fb25 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -237,6 +237,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q);
 void blk_mq_freeze_queue_start(struct request_queue *q);
 int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
 
+int blk_mq_map_queues(struct blk_mq_tag_set *set);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
 
 /*
-- 
2.1.4

Re: [PATCH 1/3] blk-mq: export blk_mq_map_queues

[Hannes Reinecke]

On 10/15/2016 10:47 AM, Christoph Hellwig wrote:
> This will allow SCSI to have a single blk_mq_ops structure that either
> lets the LLDD map the queues to PCIe MSIx vectors or use the default.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-mq.h         | 1 -
>  include/linux/blk-mq.h | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index e5d2524..5347f01 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -38,7 +38,6 @@ void blk_mq_disable_hotplug(void);
>  /*
>   * CPU -> queue mappings
>   */
> -int blk_mq_map_queues(struct blk_mq_tag_set *set);
>  extern int blk_mq_hw_queue_to_node(unsigned int *map, unsigned int);
>
>  static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 535ab2e..6c0fb25 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -237,6 +237,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q);
>  void blk_mq_freeze_queue_start(struct request_queue *q);
>  int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
>
> +int blk_mq_map_queues(struct blk_mq_tag_set *set);
>  void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
>
>  /*
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: J. Hawn, J. Guild, F. Imend�rffer, HRB 16746 (AG N�rnberg)

Re: [PATCH 1/3] blk-mq: export blk_mq_map_queues

[Johannes Thumshirn]

On Sat, Oct 15, 2016 at 10:47:19AM +0200, Christoph Hellwig wrote:
> This will allow SCSI to have a single blk_mq_ops structure that either
> lets the LLDD map the queues to PCIe MSIx vectors or use the default.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-mq.h         | 1 -
>  include/linux/blk-mq.h | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index e5d2524..5347f01 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -38,7 +38,6 @@ void blk_mq_disable_hotplug(void);
>  /*
>   * CPU -> queue mappings
>   */
> -int blk_mq_map_queues(struct blk_mq_tag_set *set);
>  extern int blk_mq_hw_queue_to_node(unsigned int *map, unsigned int);
>  
>  static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 535ab2e..6c0fb25 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -237,6 +237,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q);
>  void blk_mq_freeze_queue_start(struct request_queue *q);
>  int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
>  
> +int blk_mq_map_queues(struct blk_mq_tag_set *set);
>  void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
>  
>  /*
> -- 
> 2.1.4

See comment on patch 2, otherwise
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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