| Message ID | 20210202095832.23072-1-sreekanth.reddy@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mpt3sas: Added support for shared host tagset for cpuhotplug | expand |
Sreekanth, > MPT Fusion adapters can steer completions to individual queues, and we > now have support for shared host-wide tags. So we can enable > multiqueue support for MPT fusion adapters. Applied to 5.12/scsi-staging, thanks! -- Martin K. Petersen Oracle Linux Engineering
On 02/02/2021 09:58, Sreekanth Reddy wrote: > d transport support for SAS 3.0 HBA devices */ > @@ -12028,6 +12053,21 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) > } else > ioc->hide_drives = 0; > > + shost->host_tagset = 0; > + shost->nr_hw_queues = 1; > + > + if (ioc->is_gen35_ioc && ioc->reply_queue_count > 1 && > + host_tagset_enable && ioc->smp_affinity_enable) { > + I wanted to test host_tagset_enable feature on my LSI 3008 card (I think that it uses MP25 version), but is_gen35_ioc is not set there - is there some specific reason for which we can't support on that HW rev? > + shost->host_tagset = 1; > + shost->nr_hw_queues = > + ioc->reply_queue_count - ioc->high_iops_queues; > + > + dev_info(&ioc->pdev->dev, > + "Max SCSIIO MPT commands: %d shared with nr_hw_queues = %d\n", > + shost->can_queue, shost->nr_hw_queues); > + } Thanks, John
On Tue, Apr 20, 2021 at 8:09 PM John Garry <john.garry@huawei.com> wrote: > > On 02/02/2021 09:58, Sreekanth Reddy wrote: > > d transport support for SAS 3.0 HBA devices */ > > @@ -12028,6 +12053,21 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > } else > > ioc->hide_drives = 0; > > > > + shost->host_tagset = 0; > > + shost->nr_hw_queues = 1; > > + > > + if (ioc->is_gen35_ioc && ioc->reply_queue_count > 1 && > > + host_tagset_enable && ioc->smp_affinity_enable) { > > + > > I wanted to test host_tagset_enable feature on my LSI 3008 card (I think > that it uses MP25 version), but is_gen35_ioc is not set there - is there > some specific reason for which we can't support on that HW rev? > John, We want to avoid any code changes for older generation controllers as much as possible. Shared host tag is a major IO path change and for older generation controllers it is better not to enable such feature and support. "Fix engine policy for older controller" > > + shost->host_tagset = 1; > > + shost->nr_hw_queues = > > + ioc->reply_queue_count - ioc->high_iops_queues; > > + > > + dev_info(&ioc->pdev->dev, > > + "Max SCSIIO MPT commands: %d shared with nr_hw_queues = %d\n", > > + shost->can_queue, shost->nr_hw_queues); > > + } > > Thanks, > John
On 21/04/2021 11:20, Suganath Prabu Subramani wrote: > On Tue, Apr 20, 2021 at 8:09 PM John Garry <john.garry@huawei.com> wrote: >> >> On 02/02/2021 09:58, Sreekanth Reddy wrote: >>> d transport support for SAS 3.0 HBA devices */ >>> @@ -12028,6 +12053,21 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>> } else >>> ioc->hide_drives = 0; >>> >>> + shost->host_tagset = 0; >>> + shost->nr_hw_queues = 1; >>> + >>> + if (ioc->is_gen35_ioc && ioc->reply_queue_count > 1 && * >>> + host_tagset_enable && ioc->smp_affinity_enable) { >>> + >> >> I wanted to test host_tagset_enable feature on my LSI 3008 card (I think >> that it uses MP25 version), but is_gen35_ioc is not set there - is there >> some specific reason for which we can't support on that HW rev? >> > John, > We want to avoid any code changes for older generation controllers as much as > possible. Shared host tag is a major IO path change and for older generation > controllers it is better not to enable such feature and support. > "Fix engine policy for older controller" ok, understood. But is there a change which you can share for me to test locally? In the test for enabling, above *, I just locally removed the ioc->is_gen35_ioc condition and it looks to work ok. Is this good enough, or am I possibly doing something dangerous? I'd appreciate if you can share something for me to try locally. Thanks, John > >>> + shost->host_tagset = 1; >>> + shost->nr_hw_queues = >>> + ioc->reply_queue_count - ioc->high_iops_queues; >>> + >>> + dev_info(&ioc->pdev->dev, >>> + "Max SCSIIO MPT commands: %d shared with nr_hw_queues = %d\n", >>> + shost->can_queue, shost->nr_hw_queues); >>> + } >> >> Thanks, >> John
Hi John, Removing ioc->is_gen35_ioc condition will enable Shared host tag. We have not tested it with older generation of controllers. I will test it and let you know. Thanks, Suganath On Wed, Apr 21, 2021 at 3:57 PM John Garry <john.garry@huawei.com> wrote: > > On 21/04/2021 11:20, Suganath Prabu Subramani wrote: > > On Tue, Apr 20, 2021 at 8:09 PM John Garry <john.garry@huawei.com> wrote: > >> > >> On 02/02/2021 09:58, Sreekanth Reddy wrote: > >>> d transport support for SAS 3.0 HBA devices */ > >>> @@ -12028,6 +12053,21 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) > >>> } else > >>> ioc->hide_drives = 0; > >>> > >>> + shost->host_tagset = 0; > >>> + shost->nr_hw_queues = 1; > >>> + > >>> + if (ioc->is_gen35_ioc && ioc->reply_queue_count > 1 && > > * > > >>> + host_tagset_enable && ioc->smp_affinity_enable) { > >>> + > >> > >> I wanted to test host_tagset_enable feature on my LSI 3008 card (I think > >> that it uses MP25 version), but is_gen35_ioc is not set there - is there > >> some specific reason for which we can't support on that HW rev? > >> > > John, > > We want to avoid any code changes for older generation controllers as much as > > possible. Shared host tag is a major IO path change and for older generation > > controllers it is better not to enable such feature and support. > > "Fix engine policy for older controller" > > ok, understood. > > But is there a change which you can share for me to test locally? > > In the test for enabling, above *, I just locally removed the > ioc->is_gen35_ioc condition and it looks to work ok. Is this good > enough, or am I possibly doing something dangerous? I'd appreciate if > you can share something for me to try locally. > > Thanks, > John > > > > >>> + shost->host_tagset = 1; > >>> + shost->nr_hw_queues = > >>> + ioc->reply_queue_count - ioc->high_iops_queues; > >>> + > >>> + dev_info(&ioc->pdev->dev, > >>> + "Max SCSIIO MPT commands: %d shared with nr_hw_queues = %d\n", > >>> + shost->can_queue, shost->nr_hw_queues); > >>> + } > >> > >> Thanks, > >> John >
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index f5582c8..84663d1 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3648,25 +3648,16 @@ _base_get_msix_index(struct MPT3SAS_ADAPTER *ioc, base_mod64(atomic64_add_return(1, &ioc->total_io_cnt), ioc->reply_queue_count) : 0; - return ioc->cpu_msix_table[raw_smp_processor_id()]; -} + if (scmd && ioc->shost->nr_hw_queues > 1) { + u32 tag = blk_mq_unique_tag(scmd->request); -/** - * _base_sdev_nr_inflight_request -get number of inflight requests - * of a request queue. - * @q: request_queue object - * - * returns number of inflight request of a request queue. - */ -inline unsigned long -_base_sdev_nr_inflight_request(struct request_queue *q) -{ - struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[0]; + return blk_mq_unique_tag_to_hwq(tag) + + ioc->high_iops_queues; + } - return atomic_read(&hctx->nr_active); + return ioc->cpu_msix_table[raw_smp_processor_id()]; } - /** * _base_get_high_iops_msix_index - get the msix index of * high iops queues @@ -3686,7 +3677,8 @@ _base_get_high_iops_msix_index(struct MPT3SAS_ADAPTER *ioc, * reply queues in terms of batch count 16 when outstanding * IOs on the target device is >=8. */ - if (_base_sdev_nr_inflight_request(scmd->device->request_queue) > + + if (atomic_read(&scmd->device->device_busy) > MPT3SAS_DEVICE_HIGH_IOPS_DEPTH) return base_mod64(( atomic64_add_return(1, &ioc->high_iops_outstanding) / @@ -3739,8 +3731,23 @@ 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 = scmd->request->tag; u16 smid; + u32 tag, unique_tag; + + unique_tag = blk_mq_unique_tag(scmd->request); + tag = blk_mq_unique_tag_to_tag(unique_tag); + + /* + * store hw queue number corresponding to the tag, + * this hw queue number is used later to determine + * the unqiue_tag using below logic. This unique_tag + * is used to retrieve the scmd pointer corresponding + * to tag using scsi_host_find_tag() API, + * + * tag = smid - 1; + * unique_tag = ioc->io_queue_num[tag] << BLK_MQ_UNIQUE_TAG_BITS | tag; + */ + ioc->io_queue_num[tag] = blk_mq_unique_tag_to_hwq(unique_tag); smid = tag + 1; request->cb_idx = cb_idx; @@ -3831,6 +3838,7 @@ mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid) mpt3sas_base_clear_st(ioc, st); _base_recovery_check(ioc); + ioc->io_queue_num[smid - 1] = 0; return; } @@ -5362,6 +5370,9 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc) kfree(ioc->chain_lookup); ioc->chain_lookup = NULL; } + + kfree(ioc->io_queue_num); + ioc->io_queue_num = NULL; } /** @@ -5772,6 +5783,11 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc) ioc_info(ioc, "internal(0x%p): depth(%d), start smid(%d)\n", ioc->internal, ioc->internal_depth, ioc->internal_smid)); + + ioc->io_queue_num = kcalloc(ioc->scsiio_depth, + sizeof(u16), GFP_KERNEL); + if (!ioc->io_queue_num) + goto out; /* * The number of NVMe page sized blocks needed is: * (((sg_tablesize * 8) - 1) / (page_size - 8)) + 1 diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index 2def7a3..2eb94e4 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -1439,6 +1439,7 @@ struct MPT3SAS_ADAPTER { spinlock_t scsi_lookup_lock; int pending_io_count; wait_queue_head_t reset_wq; + u16 *io_queue_num; /* PCIe SGL */ struct dma_pool *pcie_sgl_dma_pool; diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index c8b09a8..6973041 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -54,6 +54,7 @@ #include <linux/interrupt.h> #include <linux/aer.h> #include <linux/raid_class.h> +#include <linux/blk-mq-pci.h> #include <asm/unaligned.h> #include "mpt3sas_base.h" @@ -168,6 +169,11 @@ MODULE_PARM_DESC(multipath_on_hba, "\t SAS 2.0 & SAS 3.0 HBA - This will be disabled,\n\t\t" "\t SAS 3.5 HBA - This will be enabled)"); +static int host_tagset_enable = 1; +module_param(host_tagset_enable, int, 0444); +MODULE_PARM_DESC(host_tagset_enable, + "Shared host tagset enable/disable Default: enable(1)"); + /* raid transport support */ static struct raid_template *mpt3sas_raid_template; static struct raid_template *mpt2sas_raid_template; @@ -1743,10 +1749,12 @@ mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid) struct scsi_cmnd *scmd = NULL; struct scsiio_tracker *st; Mpi25SCSIIORequest_t *mpi_request; + u16 tag = smid - 1; if (smid > 0 && smid <= ioc->scsiio_depth - INTERNAL_SCSIIO_CMDS_COUNT) { - u32 unique_tag = smid - 1; + u32 unique_tag = + ioc->io_queue_num[tag] << BLK_MQ_UNIQUE_TAG_BITS | tag; mpi_request = mpt3sas_base_get_msg_frame(ioc, smid); @@ -11599,6 +11607,22 @@ scsih_scan_finished(struct Scsi_Host *shost, unsigned long time) return 1; } +/** + * scsih_map_queues - map reply queues with request queues + * @shost: SCSI host pointer + */ +static int scsih_map_queues(struct Scsi_Host *shost) +{ + struct MPT3SAS_ADAPTER *ioc = + (struct MPT3SAS_ADAPTER *)shost->hostdata; + + if (ioc->shost->nr_hw_queues == 1) + return 0; + + return blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT], + ioc->pdev, ioc->high_iops_queues); +} + /* shost template for SAS 2.0 HBA devices */ static struct scsi_host_template mpt2sas_driver_template = { .module = THIS_MODULE, @@ -11666,6 +11690,7 @@ static struct scsi_host_template mpt3sas_driver_template = { .sdev_attrs = mpt3sas_dev_attrs, .track_queue_depth = 1, .cmd_size = sizeof(struct scsiio_tracker), + .map_queues = scsih_map_queues, }; /* raid transport support for SAS 3.0 HBA devices */ @@ -12028,6 +12053,21 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) } else ioc->hide_drives = 0; + shost->host_tagset = 0; + shost->nr_hw_queues = 1; + + if (ioc->is_gen35_ioc && ioc->reply_queue_count > 1 && + host_tagset_enable && ioc->smp_affinity_enable) { + + shost->host_tagset = 1; + shost->nr_hw_queues = + ioc->reply_queue_count - ioc->high_iops_queues; + + dev_info(&ioc->pdev->dev, + "Max SCSIIO MPT commands: %d shared with nr_hw_queues = %d\n", + shost->can_queue, shost->nr_hw_queues); + } + rv = scsi_add_host(shost, &pdev->dev); if (rv) { ioc_err(ioc, "failure at %s:%d/%s()!\n",
MPT Fusion adapters can steer completions to individual queues, and we now have support for shared host-wide tags. So we can enable multiqueue support for MPT fusion adapters. Once driver enable shared host-wide tags, cpu hotplug feature is also supported as it was enabled using below patchsets - commit bf0beec060 ("blk-mq: drain I/O when all CPUs in a hctx are offline") Currently the driver has provision to disable host-wide tags using "host_tagset_enable" module parameter. Once we do not have any major performance regression using host-wide tags, we will drop the hand-crafted interrupt affinity settings. Performance is also meeting the expectation - (used both none and mq-deadline scheduler) 24 Drive SSD on Aero with/without this patch can get 3.1M IOPs" Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> --- v2: Fix the error reported by kernel test robot drivers/scsi/mpt3sas/mpt3sas_base.c | 50 ++++++++++++++++++---------- drivers/scsi/mpt3sas/mpt3sas_base.h | 1 + drivers/scsi/mpt3sas/mpt3sas_scsih.c | 42 ++++++++++++++++++++++- 3 files changed, 75 insertions(+), 18 deletions(-)