Re: [PATCH v3] aacraid: reply queue mapping to CPUs based of IRQ affinity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16/05/2023 01:17, Sagar Biradar wrote:
Fix the IO hang that arises because of MSIx vector not
having a mapped online CPU upon receiving completion.

The SCSI cmds take the blk_mq route, which is setup during the init.
The reserved cmds fetch the vector_no from mq_map after the init
is complete and before the init, they use 0 - as per the norm.

Reviewed-by: Gilbert Wu <gilbert.wu@xxxxxxxxxxxxx>
Signed-off-by: Sagar Biradar <Sagar.Biradar@xxxxxxxxxxxxx>

this looks ok apart from minor issues, below, so fwiw:

Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>

---
  drivers/scsi/aacraid/aacraid.h  |  1 +
  drivers/scsi/aacraid/comminit.c |  2 +-
  drivers/scsi/aacraid/commsup.c  |  6 +++++-
  drivers/scsi/aacraid/linit.c    | 14 ++++++++++++++
  drivers/scsi/aacraid/src.c      | 25 +++++++++++++++++++++++--
  5 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 5e115e8b2ba4..7c6efde75da6 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1678,6 +1678,7 @@ struct aac_dev
  	u32			handle_pci_error;
  	bool			init_reset;
  	u8			soft_reset_support;
+	u8			use_map_queue;
  };
#define aac_adapter_interrupt(dev) \
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index bd99c5492b7d..53924912417e 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -630,6 +630,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
if (aac_is_src(dev))
  		aac_define_int_mode(dev);
+

stray new line

  	/*
  	 *	Ok now init the communication subsystem
  	 */
@@ -657,4 +658,3 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
return dev;
  }
-
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index deb32c9f4b3e..3f062e4013ab 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -223,8 +223,12 @@ int aac_fib_setup(struct aac_dev * dev)
  struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
  {
  	struct fib *fibptr;
+	u32 blk_tag;
+	int i;
- fibptr = &dev->fibs[scsi_cmd_to_rq(scmd)->tag];
+	blk_tag = blk_mq_unique_tag(scsi_cmd_to_rq(scmd));
+	i = blk_mq_unique_tag_to_tag(blk_tag);
+	fibptr = &dev->fibs[i];
  	/*
  	 *	Null out fields that depend on being zero at the start of
  	 *	each I/O
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 5ba5c18b77b4..fa53a9b3341b 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -34,6 +34,7 @@
  #include <linux/delay.h>
  #include <linux/kthread.h>
  #include <linux/msdos_partition.h>
+#include <linux/blk-mq-pci.h>

generally alphabetic ordering is preferred

#include <scsi/scsi.h>
  #include <scsi/scsi_cmnd.h>
@@ -505,6 +506,15 @@ static int aac_slave_configure(struct scsi_device *sdev)
  	return 0;
  }
+static void aac_map_queues(struct Scsi_Host *shost)
+{
+	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;

this pattern happens a lot in the driver, so I suggest a separate change for a helper to do this

+
+	blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
+				aac->pdev, 0);
+	aac->use_map_queue = true;
+}
+
  /**
   *	aac_change_queue_depth		-	alter queue depths
   *	@sdev:	SCSI device we are considering
@@ -1489,6 +1499,7 @@ static struct scsi_host_template aac_driver_template = {
  	.bios_param			= aac_biosparm,
  	.shost_groups			= aac_host_groups,
  	.slave_configure		= aac_slave_configure,
+	.map_queues			= aac_map_queues,
  	.change_queue_depth		= aac_change_queue_depth,
  	.sdev_groups			= aac_dev_groups,
  	.eh_abort_handler		= aac_eh_abort,
@@ -1776,6 +1787,8 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
  	shost->max_lun = AAC_MAX_LUN;
pci_set_drvdata(pdev, shost);
+	shost->nr_hw_queues = aac->max_msix;
+	shost->host_tagset = 1;
error = scsi_add_host(shost, &pdev->dev);
  	if (error)
@@ -1908,6 +1921,7 @@ static void aac_remove_one(struct pci_dev *pdev)
  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
aac_cancel_rescan_worker(aac);
+	aac->use_map_queue = false;
  	scsi_remove_host(shost);
__aac_shutdown(aac);
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 11ef58204e96..61949f374188 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -493,6 +493,10 @@ static int aac_src_deliver_message(struct fib *fib)
  #endif
u16 vector_no;
+	struct scsi_cmnd *scmd;
+	u32 blk_tag;
+	struct Scsi_Host *shost = dev->scsi_host_ptr;
+	struct blk_mq_queue_map *qmap;
atomic_inc(&q->numpending); @@ -505,8 +509,25 @@ static int aac_src_deliver_message(struct fib *fib)
  		if ((dev->comm_interface == AAC_COMM_MESSAGE_TYPE3)
  			&& dev->sa_firmware)
  			vector_no = aac_get_vector(dev);
-		else
-			vector_no = fib->vector_no;
+		else {
+			if (!fib->vector_no || !fib->callback_data) {
+				if (shost && dev->use_map_queue) {
+					qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];
+					vector_no = qmap->mq_map[raw_smp_processor_id()];
+				}
+				/*
+				 *	We hardcode the vector_no for
+				 *	reserved commands as a valid shost is
+				 *	absent during the init

you prob have another way to check this without the need for dev->use_map_queue - that's just a guess

+				 */
+				else
+					vector_no = 0;
+			} else {
+				scmd = (struct scsi_cmnd *)fib->callback_data;
+				blk_tag = blk_mq_unique_tag(scsi_cmd_to_rq(scmd));
+				vector_no = blk_mq_unique_tag_to_hwq(blk_tag);
+			}
+		}
if (native_hba) {
  			if (fib->flags & FIB_CONTEXT_FLAG_NATIVE_HBA_TMF) {




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux