| Message ID | 20231114223810.634200-1-kartilak@cisco.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce support for multiqueue (MQ) in fnic | expand |
On 14/11/2023 22:38, Karan Tilak Kumar wrote: > 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. > > Changes between v2 and v3: > Incorporate review comment from Hannes: > Replace cpy_wq_base with copy_wq_base. > Incorporate review comment from John Garry: > Replace code in fnic_mq_map_queues_cpus > with blk_mq_pci_map_queues. > Replace shost_printk logs with FNIC_MAIN_DBG. JFYI, This comment does not belong here ... > > Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com> > Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com> > Reviewed-by: Hannes Reinecke <hare@suse.de> > Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com> > --- ... should be placed here. Regardless of a couple of comments, below, feel free to pick up: Reviewed-by: John Garry <john.g.garry@oracle.com> > drivers/scsi/fnic/fnic.h | 4 +- > drivers/scsi/fnic/fnic_main.c | 109 ++++++++++++++++++++++++---------- > 2 files changed, 78 insertions(+), 35 deletions(-) > > diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h > index 97a2e547584a..5777a54c99c3 100644 > --- a/drivers/scsi/fnic/fnic.h > +++ b/drivers/scsi/fnic/fnic.h > @@ -109,7 +109,7 @@ static inline u64 fnic_flags_and_state(struct scsi_cmnd *cmd) > #define FNIC_ABT_TERM_DELAY_TIMEOUT 500 /* mSec */ > > #define FNIC_MAX_FCP_TARGET 256 > - > +#define FNIC_PCI_OFFSET 2 > /** > * state_flags to identify host state along along with fnic's state > **/ > @@ -384,7 +384,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 8bb0165490f4..93c457d88fc2 100644 > --- a/drivers/scsi/fnic/fnic_main.c > +++ b/drivers/scsi/fnic/fnic_main.c > @@ -12,9 +12,11 @@ > #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> > +#include <linux/blk-mq-pci.h> > #include <scsi/fc/fc_fip.h> > #include <scsi/scsi_host.h> > #include <scsi/scsi_transport.h> > @@ -116,6 +118,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 +395,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->copy_wq_base); > break; > default: > shost_printk(KERN_ERR, fnic->lport->host, > @@ -565,11 +568,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 +580,42 @@ static int fnic_scsi_drv_init(struct fnic *fnic) > return 0; > } > > +void fnic_mq_map_queues_cpus(struct Scsi_Host *host) > +{ > + struct fc_lport *lp = shost_priv(host); > + struct fnic *fnic = lport_priv(lp); > + struct pci_dev *l_pdev = fnic->pdev; > + int intr_mode = fnic->config.intr_mode; > + struct blk_mq_queue_map *qmap = &host->tag_set.map[HCTX_TYPE_DEFAULT]; > + > + if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) { > + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, > + "intr_mode is not msix\n"); Are these checks just paranoia? I mean that it is strange to have fnic_mq_map_queues_cpus() called but not be required to do anything. > + return; > + } > + > + FNIC_MAIN_DBG(KERN_INFO, fnic->lport->host, fnic->fnic_num, > + "qmap->nr_queues: %d\n", qmap->nr_queues); > + > + if (l_pdev == NULL) { > + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, > + "l_pdev is null\n"); > + return; > + } > + > + blk_mq_pci_map_queues(qmap, l_pdev, FNIC_PCI_OFFSET); > +} > + > 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 +632,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 +642,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 +743,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 +764,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 +858,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 +892,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 +919,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 +947,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 +960,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 +977,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 +1005,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 +1066,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); you might be able to use device-managed methods for allocating this memory, like devm_kzalloc() (so that the manual memory free'ing is not required). > fc_exch_mgr_free(fnic->lport); > vnic_dev_notify_unset(fnic->vdev); > fnic_free_intr(fnic);
On Wednesday, November 15, 2023 7:04 AM, John Garry <john.g.garry@oracle.com> wrote: > > On 14/11/2023 22:38, Karan Tilak Kumar wrote: > > 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. > > > > Changes between v2 and v3: > > Incorporate review comment from Hannes: > > Replace cpy_wq_base with copy_wq_base. > > Incorporate review comment from John Garry: > > Replace code in fnic_mq_map_queues_cpus > > with blk_mq_pci_map_queues. > > Replace shost_printk logs with FNIC_MAIN_DBG. > > JFYI, This comment does not belong here ... > > > > > Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com> > > Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com> > > Reviewed-by: Hannes Reinecke <hare@suse.de> > > Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com> > > --- > > ... should be placed here. Thanks John. I'll keep this in mind for the next time. > Regardless of a couple of comments, below, feel free to pick up: > > Reviewed-by: John Garry <john.g.garry@oracle.com> > Thanks for your review. > > +void fnic_mq_map_queues_cpus(struct Scsi_Host *host) { > > + struct fc_lport *lp = shost_priv(host); > > + struct fnic *fnic = lport_priv(lp); > > + struct pci_dev *l_pdev = fnic->pdev; > > + int intr_mode = fnic->config.intr_mode; > > + struct blk_mq_queue_map *qmap = > > +&host->tag_set.map[HCTX_TYPE_DEFAULT]; > > + > > + if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) { > > + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, > > + "intr_mode is not msix\n"); > > Are these checks just paranoia? I mean that it is strange to have > fnic_mq_map_queues_cpus() called but not be required to do anything. > Unified Computing Servers Management (UCSM) is a GUI tool to configure Cisco Servers. There are interrupt options that can be modified to INTX or MSI or MSI-x. All these options are still supported. However, we do not support multiqueue (MQ) on MSI or INTX. These checks are present to only prevent an MQ "misconfiguration". > > + for (hwq = 0; hwq < fnic->wq_copy_count; hwq++) > > + kfree(fnic->sw_copy_wq[hwq].io_req_table); > > you might be able to use device-managed methods for allocating this memory, like devm_kzalloc() (so that the manual memory free'ing is not required). > Thanks for this information. We can consider this in a future patchset. Regards, Karan
On 15/11/2023 23:20, Karan Tilak Kumar (kartilak) wrote: >>> + >>> + if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) { >>> + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, >>> + "intr_mode is not msix\n"); >> Are these checks just paranoia? I mean that it is strange to have >> fnic_mq_map_queues_cpus() called but not be required to do anything. >> > Unified Computing Servers Management (UCSM) is a GUI tool to configure Cisco Servers. > There are interrupt options that can be modified to INTX or MSI or MSI-x. > All these options are still supported. > > However, we do not support multiqueue (MQ) on MSI or INTX. By this I assume it means nr_hw_queues = 1 for MSI or INTX > These checks are present to only prevent an MQ "misconfiguration". If you check blk_mq_update_queue_map(), for set->ops->map_queues unset we call blk_mq_map_queues() - I am just wondering what does the equivalent to blk_mq_map_queues() for you in these other modes. I am concerned that it is not proper that we have a set->ops->map_queues method, but it does nothing in some scenarios. Thanks, John
On 16/11/2023 16:32, John Garry wrote: > > If you check blk_mq_update_queue_map(), for set->ops->map_queues unset > we call blk_mq_map_queues() - I am just wondering what does the > equivalent to blk_mq_map_queues() for you in these other modes. > > I am concerned that it is not proper that we have a set->ops->map_queues > method, but it does nothing in some scenarios. I think we get away with this as mq_map[for all CPUs] is 0, which points to the only HW queue. Thanks, John
On Thursday, November 16, 2023 8:33 AM, John Garry <john.g.garry@oracle.com> wrote: > > On 15/11/2023 23:20, Karan Tilak Kumar (kartilak) wrote: > >>> + > >>> + if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) { > >>> + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, > >>> + "intr_mode is not msix\n"); > >> Are these checks just paranoia? I mean that it is strange to have > >> fnic_mq_map_queues_cpus() called but not be required to do anything. > >> > > Unified Computing Servers Management (UCSM) is a GUI tool to configure Cisco Servers. > > There are interrupt options that can be modified to INTX or MSI or MSI-x. > > All these options are still supported. > > > > However, we do not support multiqueue (MQ) on MSI or INTX. > > By this I assume it means nr_hw_queues = 1 for MSI or INTX That's correct, John. > > These checks are present to only prevent an MQ "misconfiguration". > > > If you check blk_mq_update_queue_map(), for set->ops->map_queues unset we call blk_mq_map_queues() - I am just wondering what does the equivalent to blk_mq_map_queues() for you in these other modes. > > > I am concerned that it is not proper that we have a set->ops->map_queues method, but it does nothing in some scenarios. > > I think we get away with this as mq_map[for all CPUs] is 0, which points to the only HW queue. Yes, that's correct. This is what I found during testing as well. Regards, Karan
diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h index 97a2e547584a..5777a54c99c3 100644 --- a/drivers/scsi/fnic/fnic.h +++ b/drivers/scsi/fnic/fnic.h @@ -109,7 +109,7 @@ static inline u64 fnic_flags_and_state(struct scsi_cmnd *cmd) #define FNIC_ABT_TERM_DELAY_TIMEOUT 500 /* mSec */ #define FNIC_MAX_FCP_TARGET 256 - +#define FNIC_PCI_OFFSET 2 /** * state_flags to identify host state along along with fnic's state **/ @@ -384,7 +384,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 8bb0165490f4..93c457d88fc2 100644 --- a/drivers/scsi/fnic/fnic_main.c +++ b/drivers/scsi/fnic/fnic_main.c @@ -12,9 +12,11 @@ #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> +#include <linux/blk-mq-pci.h> #include <scsi/fc/fc_fip.h> #include <scsi/scsi_host.h> #include <scsi/scsi_transport.h> @@ -116,6 +118,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 +395,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->copy_wq_base); break; default: shost_printk(KERN_ERR, fnic->lport->host, @@ -565,11 +568,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 +580,42 @@ static int fnic_scsi_drv_init(struct fnic *fnic) return 0; } +void fnic_mq_map_queues_cpus(struct Scsi_Host *host) +{ + struct fc_lport *lp = shost_priv(host); + struct fnic *fnic = lport_priv(lp); + struct pci_dev *l_pdev = fnic->pdev; + int intr_mode = fnic->config.intr_mode; + struct blk_mq_queue_map *qmap = &host->tag_set.map[HCTX_TYPE_DEFAULT]; + + if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) { + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, + "intr_mode is not msix\n"); + return; + } + + FNIC_MAIN_DBG(KERN_INFO, fnic->lport->host, fnic->fnic_num, + "qmap->nr_queues: %d\n", qmap->nr_queues); + + if (l_pdev == NULL) { + FNIC_MAIN_DBG(KERN_ERR, fnic->lport->host, fnic->fnic_num, + "l_pdev is null\n"); + return; + } + + blk_mq_pci_map_queues(qmap, l_pdev, FNIC_PCI_OFFSET); +} + 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 +632,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 +642,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 +743,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 +764,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 +858,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 +892,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 +919,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 +947,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 +960,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 +977,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 +1005,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 +1066,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);
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. Changes between v2 and v3: Incorporate review comment from Hannes: Replace cpy_wq_base with copy_wq_base. Incorporate review comment from John Garry: Replace code in fnic_mq_map_queues_cpus with blk_mq_pci_map_queues. Replace shost_printk logs with FNIC_MAIN_DBG. Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com> Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com> Reviewed-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com> --- drivers/scsi/fnic/fnic.h | 4 +- drivers/scsi/fnic/fnic_main.c | 109 ++++++++++++++++++++++++---------- 2 files changed, 78 insertions(+), 35 deletions(-)