On 28/04/2023 22:07, Sagar Biradar wrote:
Fix the IO hang that arises because of MSIx vector not
having a mapped online CPU upon receiving completion.
This patch sets up a reply queue mapping to CPUs based on the
IRQ affinity retrieved using pci_irq_get_affinity() API.
aac_setup_reply_map() is an explicit mapping for internally
generated (non-SCSI) cmds.
The SCSI cmds take the blk_mq route, and the non-SCSI cmds are mapped
to the reply_map.
This now looks better.
I would still prefer if no reply_map was used even for internal
commands. As I see, you have two alternatives (to using reply_map):
- instead of using a driver-internal reply_map, lookup CPU->HW queue
mapping for internal commands by using
shost->tag_set.map[HCTX_TYPE_DEFAULT].mq_map[raw_smp_processor_id()]
Ideally when we finally support reserved commands for SCSI ML we will
have a better solution for this.
- if it is possible to send driver internal commands on a specific HW
queue always, then reserve a dedicated HW queue for them (and always
send on that HW queue). You may reserve this HW queue by omitting 1x HW
queue from pci_alloc_irq_vectors_affinity() for affinity spread
Reviewed-by: Gilbert Wu <gilbert.wu@xxxxxxxxxxxxx>
Signed-off-by: Sagar Biradar <Sagar.Biradar@xxxxxxxxxxxxx>
---
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/comminit.c | 32 ++++++++++++++++++++++++++++++++
drivers/scsi/aacraid/commsup.c | 6 +++++-
drivers/scsi/aacraid/linit.c | 25 +++++++++++++++++++++++++
drivers/scsi/aacraid/src.c | 13 +++++++++++--
5 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 5e115e8b2ba4..20f8560a3038 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;
+ unsigned int *reply_map;
};
#define aac_adapter_interrupt(dev) \
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index bd99c5492b7d..6f4e40cdaade 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -33,6 +33,8 @@
#include "aacraid.h"
+void aac_setup_reply_map(struct aac_dev *dev);
+
struct aac_common aac_config = {
.irq_mod = 1
};
@@ -630,6 +632,9 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
if (aac_is_src(dev))
aac_define_int_mode(dev);
+
+ aac_setup_reply_map(dev);
+
/*
* Ok now init the communication subsystem
*/
@@ -658,3 +663,30 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
return dev;
}
+/*
+ * aac_setup_reply_map - This is an explicit mapping for
+ * internally generated (non-SCSI) cmds which need to be
+ * serviced outside of IO requests.
+ * The SCSI cmds take the blk_mq mechanism,
+ * and the non-SCSI cmds are mapped to the reply_map.
+ */
+void aac_setup_reply_map(struct aac_dev *dev)
+{
+ const struct cpumask *mask;
+ unsigned int i, cpu = 1;
+
+ for (i = 1; i < dev->max_msix; i++) {
+ mask = pci_irq_get_affinity(dev->pdev, i);
+ if (!mask)
+ goto fallback;
+
+ for_each_cpu(cpu, mask) {
+ dev->reply_map[cpu] = i;
+ }
+ }
+ return;
+
+fallback:
+ for_each_possible_cpu(cpu)
+ dev->reply_map[cpu] = 0;
+}
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..077adbcde909 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>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -505,6 +506,16 @@ 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;
I don't think that you need a explicit casting ...
+
+ blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
+ aac->pdev, 0);
+}
+
+
+
/**
* aac_change_queue_depth - alter queue depths
* @sdev: SCSI device we are considering
@@ -1489,6 +1500,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,
@@ -1668,6 +1680,14 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
goto out_free_host;
}
+ aac->reply_map = kzalloc(sizeof(unsigned int) * nr_cpu_ids,
+ GFP_KERNEL);
+ if (!aac->reply_map) {
+ error = -ENOMEM;
+ dev_err(&pdev->dev, "reply_map allocation failed\n");
+ goto out_free_host;
+ }
+
spin_lock_init(&aac->fib_lock);
mutex_init(&aac->ioctl_mutex);
@@ -1776,6 +1796,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)
@@ -1797,6 +1819,8 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
aac->comm_addr, aac->comm_phys);
kfree(aac->queues);
aac_adapter_ioremap(aac, 0);
+ /* By now we should have configured the reply_map */
+ kfree(aac->reply_map);
kfree(aac->fibs);
kfree(aac->fsa_dev);
out_free_host:
@@ -1918,6 +1942,7 @@ static void aac_remove_one(struct pci_dev *pdev)
aac_adapter_ioremap(aac, 0);
+ kfree(aac->reply_map);
kfree(aac->fibs);
kfree(aac->fsa_dev);
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 11ef58204e96..46c0f4df995d 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -493,6 +493,8 @@ static int aac_src_deliver_message(struct fib *fib)
#endif
u16 vector_no;
+ struct scsi_cmnd *scmd;
+ u32 blk_tag;
atomic_inc(&q->numpending);
@@ -505,8 +507,15 @@ 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) {
+ vector_no = dev->reply_map[raw_smp_processor_id()];
+ } 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) {
