| Message ID | 20231130023402.802282-1-kartilak@cisco.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce support for multiqueue (MQ) in fnic | expand |
Karan, > This cover letter describes the feature: add support for multiqueue > (MQ) to fnic driver. This series doesn't apply to 6.8/scsi-queue. Also, for change entries for individual patches, please make sure you put them after a "---" separator so they don't end up in the commit history. Thanks!
On Tuesday, December 5, 2023 7:11 PM, Martin K. Petersen <martin.petersen@oracle.com> wrote: > > > Karan, > > > This cover letter describes the feature: add support for multiqueue > > (MQ) to fnic driver. > > This series doesn't apply to 6.8/scsi-queue. Okay. Thanks Martin. Please advise how I can proceed with getting the patch set accepted. > Also, for change entries for individual patches, please make sure you put them after a "---" separator so they don't end up in the commit history. > > Thanks! > > -- Thanks Martin. I'll modify the commit entries in the individual patches in the patch set and re-submit the patch set. Just to make sure I understand this correctly, this is what I will do for the individual patches in the entire patch set: 1 From 818867e64180c2456d3f33761cb7fb6e00679849 Mon Sep 17 00:00:00 2001 2 From: Karan Tilak Kumar <kartilak@cisco.com> 3 Date: Fri, 13 Oct 2023 13:38:09 -0700 4 Subject: [PATCH v4 02/13] scsi: fnic: Add and use fnic number 5 6 Add fnic_num in fnic.h to identify fnic in a multi-fnic environment. 7 Increment and set the fnic number during driver load in fnic_probe. 8 Replace the host number with fnic number in debugfs. 9 10 Reviewed-by: Sesidhar Baddela <sebaddel@cisco.com> 11 Reviewed-by: Arulprabhu Ponnusamy <arulponn@cisco.com> 12 Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com> 13 --- -> Introduce a separator between the commit and the change entries 14 Changes between v3 and v4: 15 Incorporate review comments from Martin and Hannes: 16 Undo the change to replace host number with fnic 17 number in debugfs since kernel stack uses host number. 18 Use ida_alloc to allocate ID for fnic number. Please let me know if I've missed anything. Regards, Karan
Hi Karan! >> This series doesn't apply to 6.8/scsi-queue. > > Okay. Thanks Martin. > Please advise how I can proceed with getting the patch set accepted. I tried v5 and it fails the same way as v4. First failure is patch #9, several additional conflicts after that. You need to submit a patch series that has been rebased on top of my 6.8/scsi-queue branch. I don't know what you used as baseline for the series in your git tree but it is not a suitable ancestor of my tree. Your changelog modification is fine. Thanks!
On Friday, December 8, 2023 10:04 AM, Martin K. Petersen <martin.petersen@oracle.com> wrote: > > > Hi Karan! > > >> This series doesn't apply to 6.8/scsi-queue. > > > > Okay. Thanks Martin. > > Please advise how I can proceed with getting the patch set accepted. > > I tried v5 and it fails the same way as v4. First failure is patch #9, several additional conflicts after that. > > You need to submit a patch series that has been rebased on top of my 6.8/scsi-queue branch. I don't know what you used as baseline for the series in your git tree but it is not a suitable ancestor of my tree. > > Your changelog modification is fine. Thanks! > > -- > Martin K. Petersen Oracle Linux Engineering > Thanks for this crucial piece of information, Martin. I'll rebase the changes on top of the mentioned branch and re-submit the patch set. Regards, Karan
Hi Martin, reviewers, This cover letter describes the feature: add support for multiqueue (MQ) to fnic driver. Background: The Virtual Interface Card (VIC) firmware exposes several queues that can be configured for sending IOs and receiving IO responses. Unified Computing System Manager (UCSM) and Intersight Manager (IMM) allows users to configure the number of queues to be used for IOs. The number of IO queues to be used is stored in a configuration file by the VIC firmware. The fNIC driver reads the configuration file and sets the number of queues to be used. Previously, the driver was hard-coded to use only one queue. With this set of changes, the fNIC driver will configure itself to use multiple queues. This feature takes advantage of the block multiqueue layer to parallelize IOs being sent out of the VIC card. Here's a brief description of some of the salient patches: - vnic_scsi.h needs to be in sync with VIC firmware to be able to read the number of queues from the firmware config file. A patch has been created for this. - In an environment with many fnics (like we see in our customer environments), it is hard to distinguish which fnic is printing logs. Therefore, an fnic number has been included in the logs. - read the number of queues from the firmware config file. - include definitions in fnic.h to support multiqueue. - modify the interrupt service routines (ISRs) to read from the correct registers. The numbers that are used here come from discussions with the VIC firmware team. - track IO statistics for different queues. - remove usage of host_lock, and only use fnic_lock in the fnic driver. - use a hardware queue based spinlock to protect io_req. - replace the hard-coded zeroth queue with a hardware queue number. This presents a bulk of the changes. - modify the definition of fnic_queuecommand to accept multiqueue tags. - improve log messages, and indicate fnic number and multiqueue tags for effective debugging. Even though the patches have been made into a series, some patches are heavier than others. But, every effort has been made to keep the purpose of each patch as a single-purpose, and to compile cleanly. This patchset has been tested as a whole. Therefore, the tested-by fields have been added only to two patches in the set. All the individual patches compile cleanly. However, I've refrained from adding tested-by to most of the patches, so as to not mislead the reviewer/reader. A brief note on the unit tests: 1. Increase number of queues to 64. Load driver. Run IOs via Medusa. 12+ hour run successful. 2. Configure multipathing, and run link flaps on single link. IOs drop briefly, but pick up as expected. 3. Configure multipathing, and run link flaps on two links, with a 30 second delay in between. IOs drop briefly, but pick up as expected. Repeat the above tests with 1 queue and 32 queues. All tests were successful. Please consider this patch series for the next merge window. Changes between v1 and v2: Suppress a warning raised by a kernel test bot, Incorporate the following review comments from Bart: Remove outdated comment, Remove unnecessary out of range tag checks, Remove unnecessary local variable, Modify function name. Changes between v2 and v3: Incorporate review comment from Hannes: Modify FNIC_MAIN_DBG to prepend fnic number. Modify FNIC_MAIN_DBG definition to prepend function name and line number. Modify FNIC_FCS_DBG definition to prepend function name and line number. Replace FNIC_MAIN_DBG with FNIC_FCS_DBG in fnic_fcs.c Use fnic_num as an argument to FNIC_MAIN_DBG and FNIC_FCS_DBG. Host number is still used as an argument to FNIC_MAIN_DBG and FNIC_FCS_DBG since it in turn uses shost_printk. Replace cpy_wq_base with copy_wq_base. Incorporate the following review comments from Hannes: Replace cpy_wq_base with copy_wq_base. Remove C99 style comment. Extend review comments of FNIC_MAIN_DBG and FNIC_SCSI_DBG to FNIC_ISR_DBG: Use fnic_num as an argument to FNIC_ISR_DBG. Modify definition of FNIC_ISR_DBG. Host number is still used as an argument to FNIC_ISR_DBG since it in turn uses shost_printk. Removed reviewed by tag from Hannes due to additional modifications. Squash the following commits into one: scsi: fnic: Remove usage of host_lock scsi: fnic: Use fnic_lock to protect fnic structures in queuecommand 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. Incorporate the following review comments from Hannes: Replace cpy_wq_base with copy_wq_base. Remove hwq as an argument to fnic_queuecommand_int. Suppress warning from kernel test robot. Replace new shost_printk comments with FNIC_SCSI_DBG. Replace fnic_queuecommand_int with fnic_queuecommand. Incorporate the following review comment from Hannes: Use fnic_num as an argument to FNIC_SCSI_DBG. Modify definition of FNIC_SCSI_DBG. Host number is still an argument since FNIC_SCSI_DBG in turn uses shost_printk. Create a separate patch to increment driver version. Increment driver version number to 1.7.0.0. Changes between v3 and v4: Incorporate review comments from Martin and Hannes: Undo the change to replace host number with fnic number in debugfs since kernel stack uses host number. Use ida_alloc to allocate ID for fnic number. Thanks and regards, Karan Karan Tilak Kumar (13): scsi: fnic: Modify definitions to sync with VIC firmware scsi: fnic: Add and use fnic number scsi: fnic: Add and improve log messages scsi: fnic: Rename wq_copy to hw_copy_wq scsi: fnic: Get copy workqueue count and interrupt mode from config scsi: fnic: Refactor and redefine fnic.h for multiqueue scsi: fnic: Modify ISRs to support multiqueue(MQ) scsi: fnic: Define stats to track multiqueue (MQ) IOs scsi: fnic: Remove usage of host_lock scsi: fnic: Add support for multiqueue (MQ) in fnic_main.c scsi: fnic: Add support for multiqueue (MQ) in fnic driver scsi: fnic: Improve logs and add support for multiqueue (MQ) scsi: fnic: Increment driver version drivers/scsi/fnic/fnic.h | 68 ++- drivers/scsi/fnic/fnic_fcs.c | 63 +-- drivers/scsi/fnic/fnic_io.h | 2 + drivers/scsi/fnic/fnic_isr.c | 168 +++++-- drivers/scsi/fnic/fnic_main.c | 142 ++++-- drivers/scsi/fnic/fnic_res.c | 48 +- drivers/scsi/fnic/fnic_scsi.c | 862 +++++++++++++++++++-------------- drivers/scsi/fnic/fnic_stats.h | 3 + drivers/scsi/fnic/fnic_trace.c | 11 + drivers/scsi/fnic/vnic_dev.c | 4 + drivers/scsi/fnic/vnic_scsi.h | 13 +- 11 files changed, 860 insertions(+), 524 deletions(-)