[v2,00/13] Introduce support for multiqueue (MQ) in fnic

Message

Karan Tilak Kumar (kartilak) Oct. 27, 2023, 6:02 p.m. UTC

Hi Martin,

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 the last 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 single queue. 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.
 
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: Use fnic_lock to protect fnic structures in queuecommand
  scsi: fnic: Add support for multiqueue (MQ) in fnic driver
  scsi: fnic: Improve logs and add support for multiqueue (MQ)

 drivers/scsi/fnic/fnic.h         |  42 +-
 drivers/scsi/fnic/fnic_debugfs.c |   2 +-
 drivers/scsi/fnic/fnic_fcs.c     |  36 +-
 drivers/scsi/fnic/fnic_io.h      |   2 +
 drivers/scsi/fnic/fnic_isr.c     | 166 +++++--
 drivers/scsi/fnic/fnic_main.c    | 156 ++++--
 drivers/scsi/fnic/fnic_res.c     |  48 +-
 drivers/scsi/fnic/fnic_scsi.c    | 789 ++++++++++++++++++-------------
 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 +-
 12 files changed, 823 insertions(+), 449 deletions(-)

Comments

John Garry Nov. 2, 2023, 9 a.m. UTC | #1

On 27/10/2023 19:02, Karan Tilak Kumar wrote:
> 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.

In looking at commit aec95e3a8ded ("scsi: fnic: Refactor code in fnic 
probe to initialize SCSI layer"), support to set shost->nr_hw_queues > 1 
to enable MQ seems to have sneaked in already - what's going on there?

John

Karan Tilak Kumar (kartilak) Nov. 6, 2023, 7:44 p.m. UTC | #2

On Thursday, November 2, 2023 2:00 AM, John Garry <[e-mail]> wrote:
>
> On 27/10/2023 19:02, Karan Tilak Kumar wrote:
> > 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.
>
> In looking at commit aec95e3a8ded ("scsi: fnic: Refactor code in fnic
> probe to initialize SCSI layer"), support to set shost->nr_hw_queues > 1
> to enable MQ seems to have sneaked in already - what's going on there?
>
> John

Thanks for your review comments, John.
This was a defensive check and code to show that fnic does not have support for MQ 
when the user tries to configure multiple IO queues.
The support for MQ is being added only now.

Regards,
Karan