On Wednesday, November 15, 2023 7:04 AM, John Garry <john.g.garry@xxxxxxxxxx> 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@xxxxxxxxx>
> > Reviewed-by: Arulprabhu Ponnusamy <arulponn@xxxxxxxxx>
> > Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
> > Signed-off-by: Karan Tilak Kumar <kartilak@xxxxxxxxx>
> > ---
>
> ... 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@xxxxxxxxxx>
>
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
