[10/13] scsi: fnic: Add support for multiqueue (MQ) in fnic_main.c

Message ID 20231020190629.338623-11-kartilak@cisco.com
State New
Headers
Series Introduce support for multiqueue (MQ) in |

Commit Message

Karan Tilak Kumar (kartilak) Oct. 20, 2023, 7:06 p.m. UTC
  Set map_queues in the fnic_host_template to fnic_mq_map_queues_cpus.
Define fnic_mq_map_queues_cpus to set cpu assignment to
fnic queues.
Refactor code in fnic_probe to enable vnic queues before scsi_add_host.
Modify notify set to the correct index.

Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
---
 drivers/scsi/fnic/fnic.h      |   2 +-
 drivers/scsi/fnic/fnic_main.c | 137 ++++++++++++++++++++++++++--------
 2 files changed, 105 insertions(+), 34 deletions(-)
  

Comments

John Garry Nov. 8, 2023, 1:57 p.m. UTC | #1
>   static void
> @@ -392,7 +394,7 @@ static int fnic_notify_set(struct fnic *fnic)
>   		err = vnic_dev_notify_set(fnic->vdev, -1);
>   		break;
>   	case VNIC_DEV_INTR_MODE_MSIX:
> -		err = vnic_dev_notify_set(fnic->vdev, FNIC_MSIX_ERR_NOTIFY);
> +		err = vnic_dev_notify_set(fnic->vdev, fnic->wq_copy_count + fnic->cpy_wq_base);
>   		break;
>   	default:
>   		shost_printk(KERN_ERR, fnic->lport->host,
> @@ -565,11 +567,6 @@ static int fnic_scsi_drv_init(struct fnic *fnic)
>   	host->max_cmd_len = FCOE_MAX_CMD_LEN;
>   
>   	host->nr_hw_queues = fnic->wq_copy_count;

Please be aware of comment on nr_hw_queues in scsi_host.h - maybe it is 
relevant to this adapter:

"the total queue depth per host is nr_hw_queues * can_queue"

Also, since you seem to be using blk_mq_unique_tag() as the per-IO tag, 
I assume that you don't need to set shost.host_tagset (for that reason).

> -	if (host->nr_hw_queues > 1)
> -		shost_printk(KERN_ERR, host,
> -				"fnic: blk-mq is not supported");
> -
> -	host->nr_hw_queues = fnic->wq_copy_count = 1;
>   
>   	shost_printk(KERN_INFO, host,
>   			"fnic: can_queue: %d max_lun: %llu",
> @@ -582,15 +579,71 @@ static int fnic_scsi_drv_init(struct fnic *fnic)
>   	return 0;
>   }
>   
> +void fnic_mq_map_queues_cpus(struct Scsi_Host *host)

This function looks to do effectively the same as 
blk_mq_pci_map_queues(), right?

> +{
> +	const struct cpumask *mask;
> +	unsigned int queue, cpu;
> +	int irq_num;
> +	struct fc_lport *lp = shost_priv(host);
> +	struct fnic *fnic = lport_priv(lp);
> +	struct pci_dev *l_pdev = fnic->pdev;
> +	struct blk_mq_tag_set *set = &host->tag_set;
> +	int intr_mode = fnic->config.intr_mode;
> +
> +	if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) {
> +		shost_printk(KERN_ERR, fnic->lport->host,
> +			"fnic<%d>: %s: %d: intr_mode is not msix\n",
> +			fnic->fnic_num, __func__, __LINE__);
> +		return;
> +	}
> +
> +	shost_printk(KERN_INFO, fnic->lport->host,
> +			"fnic<%d>: %s: %d: set->nr_hw_queues: %d\n",
> +			fnic->fnic_num, __func__, __LINE__, set->nr_hw_queues);
> +
> +	for (queue = 0; queue < set->nr_hw_queues; queue++) {
> +		if (l_pdev == NULL) {
> +			shost_printk(KERN_ERR, fnic->lport->host,
> +				"fnic<%d>: %s: %d: l_pdev is null\n",
> +				fnic->fnic_num, __func__, __LINE__);
> +			return;
> +		}
> +
> +		irq_num = pci_irq_vector(l_pdev, queue+2);
> +		if (irq_num < 0) {
> +			shost_printk(KERN_ERR, fnic->lport->host,
> +				"fnic<%d>: %s: %d: irq_num less than zero: %d\n",
> +				fnic->fnic_num, __func__, __LINE__, irq_num);
> +			continue;
> +		}
> +
> +		mask = irq_get_effective_affinity_mask(irq_num);
> +		if (!mask) {
> +			shost_printk(KERN_ERR, fnic->lport->host,
> +				"fnic<%d>: %s: %d: failed to get irq_affinity map for queue: %d\n",
> +				fnic->fnic_num, __func__, __LINE__, irq_num);
> +			continue;
> +		}
> +
> +		for_each_cpu(cpu, mask) {
> +			set->map[HCTX_TYPE_DEFAULT].mq_map[cpu] = queue;
> +			shost_printk(KERN_INFO, fnic->lport->host,
> +				"fnic<%d>: %s: %d: cpu: %d <=> queue: %d\n",
> +				fnic->fnic_num, __func__, __LINE__, cpu, irq_num);
> +		}
> +	}
> +}
> +

Thanks,
John
  
Karan Tilak Kumar (kartilak) Nov. 9, 2023, 2:21 a.m. UTC | #2
On Wednesday, November 8, 2023 5:57 AM, John Garry <john.g.garry@oracle.com> wrote:
> Please be aware of comment on nr_hw_queues in scsi_host.h - maybe it is relevant to this adapter:
>
> "the total queue depth per host is nr_hw_queues * can_queue"

Thanks John.

> Also, since you seem to be using blk_mq_unique_tag() as the per-IO tag, I assume that you don't need to set shost.host_tagset (for that reason).

Yes, that's correct.

> > +void fnic_mq_map_queues_cpus(struct Scsi_Host *host)
>
> This function looks to do effectively the same as blk_mq_pci_map_queues(), right?

Yes, it looks like it is accomplishing the same goal. Thanks for this information.
I'll experiment with this function on my setup and make suitable modifications in v3, if successful.

Regards,
Karan
  

Patch

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 692175cfa557..182194b6d51d 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -377,7 +377,7 @@  void fnic_wq_copy_cleanup_handler(struct vnic_wq_copy *wq,
 int fnic_fw_reset_handler(struct fnic *fnic);
 void fnic_terminate_rport_io(struct fc_rport *);
 const char *fnic_state_to_str(unsigned int state);
-
+void fnic_mq_map_queues_cpus(struct Scsi_Host *host);
 void fnic_log_q_error(struct fnic *fnic);
 void fnic_handle_link_event(struct fnic *fnic);
 
diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 66e85754c1d6..5b60396e7349 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -12,6 +12,7 @@ 
 #include <linux/pci.h>
 #include <linux/skbuff.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <linux/if_ether.h>
@@ -116,6 +117,7 @@  static const struct scsi_host_template fnic_host_template = {
 	.shost_groups = fnic_host_groups,
 	.track_queue_depth = 1,
 	.cmd_size = sizeof(struct fnic_cmd_priv),
+	.map_queues = fnic_mq_map_queues_cpus,
 };
 
 static void
@@ -392,7 +394,7 @@  static int fnic_notify_set(struct fnic *fnic)
 		err = vnic_dev_notify_set(fnic->vdev, -1);
 		break;
 	case VNIC_DEV_INTR_MODE_MSIX:
-		err = vnic_dev_notify_set(fnic->vdev, FNIC_MSIX_ERR_NOTIFY);
+		err = vnic_dev_notify_set(fnic->vdev, fnic->wq_copy_count + fnic->cpy_wq_base);
 		break;
 	default:
 		shost_printk(KERN_ERR, fnic->lport->host,
@@ -565,11 +567,6 @@  static int fnic_scsi_drv_init(struct fnic *fnic)
 	host->max_cmd_len = FCOE_MAX_CMD_LEN;
 
 	host->nr_hw_queues = fnic->wq_copy_count;
-	if (host->nr_hw_queues > 1)
-		shost_printk(KERN_ERR, host,
-				"fnic: blk-mq is not supported");
-
-	host->nr_hw_queues = fnic->wq_copy_count = 1;
 
 	shost_printk(KERN_INFO, host,
 			"fnic: can_queue: %d max_lun: %llu",
@@ -582,15 +579,71 @@  static int fnic_scsi_drv_init(struct fnic *fnic)
 	return 0;
 }
 
+void fnic_mq_map_queues_cpus(struct Scsi_Host *host)
+{
+	const struct cpumask *mask;
+	unsigned int queue, cpu;
+	int irq_num;
+	struct fc_lport *lp = shost_priv(host);
+	struct fnic *fnic = lport_priv(lp);
+	struct pci_dev *l_pdev = fnic->pdev;
+	struct blk_mq_tag_set *set = &host->tag_set;
+	int intr_mode = fnic->config.intr_mode;
+
+	if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) {
+		shost_printk(KERN_ERR, fnic->lport->host,
+			"fnic<%d>: %s: %d: intr_mode is not msix\n",
+			fnic->fnic_num, __func__, __LINE__);
+		return;
+	}
+
+	shost_printk(KERN_INFO, fnic->lport->host,
+			"fnic<%d>: %s: %d: set->nr_hw_queues: %d\n",
+			fnic->fnic_num, __func__, __LINE__, set->nr_hw_queues);
+
+	for (queue = 0; queue < set->nr_hw_queues; queue++) {
+		if (l_pdev == NULL) {
+			shost_printk(KERN_ERR, fnic->lport->host,
+				"fnic<%d>: %s: %d: l_pdev is null\n",
+				fnic->fnic_num, __func__, __LINE__);
+			return;
+		}
+
+		irq_num = pci_irq_vector(l_pdev, queue+2);
+		if (irq_num < 0) {
+			shost_printk(KERN_ERR, fnic->lport->host,
+				"fnic<%d>: %s: %d: irq_num less than zero: %d\n",
+				fnic->fnic_num, __func__, __LINE__, irq_num);
+			continue;
+		}
+
+		mask = irq_get_effective_affinity_mask(irq_num);
+		if (!mask) {
+			shost_printk(KERN_ERR, fnic->lport->host,
+				"fnic<%d>: %s: %d: failed to get irq_affinity map for queue: %d\n",
+				fnic->fnic_num, __func__, __LINE__, irq_num);
+			continue;
+		}
+
+		for_each_cpu(cpu, mask) {
+			set->map[HCTX_TYPE_DEFAULT].mq_map[cpu] = queue;
+			shost_printk(KERN_INFO, fnic->lport->host,
+				"fnic<%d>: %s: %d: cpu: %d <=> queue: %d\n",
+				fnic->fnic_num, __func__, __LINE__, cpu, irq_num);
+		}
+	}
+}
+
 static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct Scsi_Host *host;
 	struct fc_lport *lp;
 	struct fnic *fnic;
 	mempool_t *pool;
-	int err;
+	int err = 0;
 	int i;
 	unsigned long flags;
+	int hwq;
 
 	atomic_inc(&fnic_num);
 	/*
@@ -607,8 +660,8 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	fnic = lport_priv(lp);
 	fnic->lport = lp;
 	fnic->ctlr.lp = lp;
-
 	fnic->link_events = 0;
+	fnic->pdev = pdev;
 
 	snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
 		 host->host_no);
@@ -617,11 +670,6 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	fnic->fnic_num = atomic_read(&fnic_num);
 	fnic_stats_debugfs_init(fnic);
 
-	/* Setup PCI resources */
-	pci_set_drvdata(pdev, fnic);
-
-	fnic->pdev = pdev;
-
 	err = pci_enable_device(pdev);
 	if (err) {
 		shost_printk(KERN_ERR, fnic->lport->host,
@@ -723,7 +771,8 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_out_dev_close;
 	}
 
-	fnic_scsi_drv_init(fnic);
+	/* Setup PCI resources */
+	pci_set_drvdata(pdev, fnic);
 
 	fnic_get_res_counts(fnic);
 
@@ -743,6 +792,16 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_out_clear_intr;
 	}
 
+	fnic_scsi_drv_init(fnic);
+
+	for (hwq = 0; hwq < fnic->wq_copy_count; hwq++) {
+		fnic->sw_copy_wq[hwq].ioreq_table_size = fnic->fnic_max_tag_id;
+		fnic->sw_copy_wq[hwq].io_req_table =
+					kzalloc((fnic->sw_copy_wq[hwq].ioreq_table_size + 1) *
+					sizeof(struct fnic_io_req *), GFP_KERNEL);
+	}
+	shost_printk(KERN_INFO, fnic->lport->host, "fnic copy wqs: %d, Q0 ioreq table size: %d\n",
+			fnic->wq_copy_count, fnic->sw_copy_wq[0].ioreq_table_size);
 
 	/* initialize all fnic locks */
 	spin_lock_init(&fnic->fnic_lock);
@@ -827,16 +886,32 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	/* allocate RQ buffers and post them to RQ*/
 	for (i = 0; i < fnic->rq_count; i++) {
-		vnic_rq_enable(&fnic->rq[i]);
 		err = vnic_rq_fill(&fnic->rq[i], fnic_alloc_rq_frame);
 		if (err) {
 			shost_printk(KERN_ERR, fnic->lport->host,
 				     "fnic_alloc_rq_frame can't alloc "
 				     "frame\n");
-			goto err_out_free_rq_buf;
+			goto err_out_rq_buf;
 		}
 	}
 
+	/* Enable all queues */
+	for (i = 0; i < fnic->raw_wq_count; i++)
+		vnic_wq_enable(&fnic->wq[i]);
+	for (i = 0; i < fnic->rq_count; i++) {
+		if (!ioread32(&fnic->rq[i].ctrl->enable))
+			vnic_rq_enable(&fnic->rq[i]);
+	}
+	for (i = 0; i < fnic->wq_copy_count; i++)
+		vnic_wq_copy_enable(&fnic->hw_copy_wq[i]);
+
+	err = fnic_request_intr(fnic);
+	if (err) {
+		shost_printk(KERN_ERR, fnic->lport->host,
+			     "Unable to request irq.\n");
+		goto err_out_request_intr;
+	}
+
 	/*
 	 * Initialization done with PCI system, hardware, firmware.
 	 * Add host to SCSI
@@ -845,9 +920,10 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err) {
 		shost_printk(KERN_ERR, fnic->lport->host,
 			     "fnic: scsi_add_host failed...exiting\n");
-		goto err_out_free_rq_buf;
+		goto err_out_scsi_add_host;
 	}
 
+
 	/* Start local port initiatialization */
 
 	lp->link_up = 0;
@@ -871,7 +947,7 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!fc_exch_mgr_alloc(lp, FC_CLASS_3, FCPIO_HOST_EXCH_RANGE_START,
 			       FCPIO_HOST_EXCH_RANGE_END, NULL)) {
 		err = -ENOMEM;
-		goto err_out_remove_scsi_host;
+		goto err_out_fc_exch_mgr_alloc;
 	}
 
 	fc_lport_init_stats(lp);
@@ -899,21 +975,8 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	skb_queue_head_init(&fnic->frame_queue);
 	skb_queue_head_init(&fnic->tx_queue);
 
-	/* Enable all queues */
-	for (i = 0; i < fnic->raw_wq_count; i++)
-		vnic_wq_enable(&fnic->wq[i]);
-	for (i = 0; i < fnic->wq_copy_count; i++)
-		vnic_wq_copy_enable(&fnic->hw_copy_wq[i]);
-
 	fc_fabric_login(lp);
 
-	err = fnic_request_intr(fnic);
-	if (err) {
-		shost_printk(KERN_ERR, fnic->lport->host,
-			     "Unable to request irq.\n");
-		goto err_out_free_exch_mgr;
-	}
-
 	vnic_dev_enable(fnic->vdev);
 
 	for (i = 0; i < fnic->intr_count; i++)
@@ -925,12 +988,15 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 err_out_free_exch_mgr:
 	fc_exch_mgr_free(lp);
-err_out_remove_scsi_host:
+err_out_fc_exch_mgr_alloc:
 	fc_remove_host(lp->host);
 	scsi_remove_host(lp->host);
-err_out_free_rq_buf:
+err_out_scsi_add_host:
+	fnic_free_intr(fnic);
+err_out_request_intr:
 	for (i = 0; i < fnic->rq_count; i++)
 		vnic_rq_clean(&fnic->rq[i], fnic_free_rq_buf);
+err_out_rq_buf:
 	vnic_dev_notify_unset(fnic->vdev);
 err_out_free_max_pool:
 	mempool_destroy(fnic->io_sgl_pool[FNIC_SGL_CACHE_MAX]);
@@ -939,6 +1005,8 @@  static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_out_free_ioreq_pool:
 	mempool_destroy(fnic->io_req_pool);
 err_out_free_resources:
+	for (hwq = 0; hwq < fnic->wq_copy_count; hwq++)
+		kfree(fnic->sw_copy_wq[hwq].io_req_table);
 	fnic_free_vnic_resources(fnic);
 err_out_clear_intr:
 	fnic_clear_intr_mode(fnic);
@@ -965,6 +1033,7 @@  static void fnic_remove(struct pci_dev *pdev)
 	struct fnic *fnic = pci_get_drvdata(pdev);
 	struct fc_lport *lp = fnic->lport;
 	unsigned long flags;
+	int hwq;
 
 	/*
 	 * Mark state so that the workqueue thread stops forwarding
@@ -1025,6 +1094,8 @@  static void fnic_remove(struct pci_dev *pdev)
 
 	fc_remove_host(fnic->lport->host);
 	scsi_remove_host(fnic->lport->host);
+	for (hwq = 0; hwq < fnic->wq_copy_count; hwq++)
+		kfree(fnic->sw_copy_wq[hwq].io_req_table);
 	fc_exch_mgr_free(fnic->lport);
 	vnic_dev_notify_unset(fnic->vdev);
 	fnic_free_intr(fnic);