Message ID | 97dcf365-89ff-014d-a3e5-1404c6af511c@cybernetics.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | scsi: qla2xxx: Disable ATIO interrupt coalesce for quad port ISP27XX | expand |
Quinn/Nilesh: Please review, thanks! > I am using a quad-port 16 Gbps QLE2694L (ISP2071) with SCST 3.6 + > qla2x00t-32gbit and kernel 5.15. The following old commit that went > into upstream kernel v4.16 is causing a problem for me: > > commit d2b292c3f6fdef5819a276acd64915bae6384a7f > Author: Quinn Tran <quinn.tran@cavium.com> > Date: Thu Dec 28 12:33:17 2017 -0800 > > scsi: qla2xxx: Enable ATIO interrupt handshake for ISP27XX > > Enable ATIO Q interrupt handshake for ISP27XX. This patch > coalesce ATIO's interrupts for Quad port ISP27XX adapter. > Interrupt coalesce allows performance to scale for this > specific case. > > This commit was present in qla2xxx when qla2x00t-32gbit was first > imported into SCST.
> On Jul 20, 2022, at 6:43 AM, Tony Battersby <tonyb@cybernetics.com> wrote: > > This message was previously sent to scst-devel: > https://sourceforge.net/p/scst/mailman/scst-devel/thread/d381eb86-d446-4f9e-03c3-aa93d93dd074%40cybernetics.com/#msg37682919 > > I am using a quad-port 16 Gbps QLE2694L (ISP2071) with SCST 3.6 + > qla2x00t-32gbit and kernel 5.15. The following old commit that went > into upstream kernel v4.16 is causing a problem for me: > > commit d2b292c3f6fdef5819a276acd64915bae6384a7f > Author: Quinn Tran <quinn.tran@cavium.com> > Date: Thu Dec 28 12:33:17 2017 -0800 > > scsi: qla2xxx: Enable ATIO interrupt handshake for ISP27XX > > Enable ATIO Q interrupt handshake for ISP27XX. This patch > coalesce ATIO's interrupts for Quad port ISP27XX adapter. > Interrupt coalesce allows performance to scale for this > specific case. > > This commit was present in qla2xxx when qla2x00t-32gbit was first > imported into SCST. > > With the "ATIO interrupt coalesce" enabled by the above patch, ATIO > interrupts are handled by qla24xx_msix_default() rather than by > qla83xx_msix_atio_q() for the ISP2071. I guess this is supposed to > generate fewer interrupts so that ATIO entries can be handled in larger > batches. But this causes a problem where sometimes > qlt_24xx_process_atio_queue() returns while the hardware is still adding > ATIO entries to the queue, but then the hardware doesn't generate > another interrupt until a separate event sometimes minutes later. This > leaves incoming ATIO entries (i.e. incoming target-mode SCSI commands) > ignored in the adapter's hardware queue for a long time until the host > sends another command at a different time to generate an interrupt. > There does not seem to be any timeout function that generates an > interrupt after a short period of inactivity to process the remainder of > the ATIO queue, which is what would be necessary for interrupt coalesce > to function properly. > > In my case I am using virtual tape drives, which generally process only > one command at a time, so there will often never be a "next" command to > generate an interrupt until the previous command completes. If the > previous command is stuck in the adapter's ATIO queue, then the host > will timeout and abort the command, and the task management function to > abort the command generates the interrupt that causes the command to > finally be received by SCST, so the command appears to have been aborted > immediately. With disk drives the situation might be a bit different > since a busy disk might get a steady stream of commands to generate > additional interrupts to process the ATIO queue, but even that is not > guaranteed, so there might still be problems. > > I tried to fix the problem by modifying qla24xx_msix_default() to call > qlt_24xx_process_atio_queue() before wrt_reg_dword(®->hccr, > HCCRX_CLR_RISC_INT) instead of after, but that didn't help. The problem > was solved by disabling the ATIO interrupt coalesce feature and > re-enabling the dedicated ATIO MSI-X interrupt (qla83xx_msix_atio_q()) > for calling qlt_24xx_process_atio_queue(). Another workaround is to use > ql2xenablemsix=0, but of course keeping MSI-X enabled is better. > > The patch below is against the upstream Linux kernel. It can be applied > to SCST with "patch -p4" from the qla2x00t-32gbit directory. > > If someone really wants to keep the interrupt coalesce feature, it could > be turned into a module parameter instead. > > From e5888ce5dbffff2d38d9dafd4841d6d3bcda4365 Mon Sep 17 00:00:00 2001 > From: Tony Battersby <tonyb@cybernetics.com> > Date: Thu, 7 Jul 2022 15:08:01 -0400 > Subject: [PATCH] scsi: qla2xxx: Disable ATIO interrupt coalesce for quad port ISP27XX > > This partially reverts commit d2b292c3f6fdef5819a276acd64915bae6384a7f. > > For some workloads where the host sends a batch of commands and then > pauses, ATIO interrupt coalesce can cause some incoming ATIO entries to > be ignored for extended periods of time, resulting in slow performance, > timeouts, and aborted commands. So disable interrupt coalesce and > re-enable the dedicated ATIO MSI-X interrupt. > > Signed-off-by: Tony Battersby <tonyb@cybernetics.com> > --- > drivers/scsi/qla2xxx/qla_target.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c > index cb97f625970d..80fd9980dfc9 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -6932,14 +6932,8 @@ qlt_24xx_config_rings(struct scsi_qla_host *vha) > > if (ha->flags.msix_enabled) { > if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) { > - if (IS_QLA2071(ha)) { > - /* 4 ports Baker: Enable Interrupt Handshake */ > - icb->msix_atio = 0; > - icb->firmware_options_2 |= cpu_to_le32(BIT_26); > - } else { > - icb->msix_atio = cpu_to_le16(msix->entry); > - icb->firmware_options_2 &= cpu_to_le32(~BIT_26); > - } > + icb->msix_atio = cpu_to_le16(msix->entry); > + icb->firmware_options_2 &= cpu_to_le32(~BIT_26); > ql_dbg(ql_dbg_init, vha, 0xf072, > "Registering ICB vector 0x%x for atio que.\n", > msix->entry); > Looks Good. Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com> -- Himanshu Madhani Oracle Linux Engineering
> -----Original Message----- > From: Tony Battersby <tonyb@cybernetics.com> > Sent: Wednesday, July 20, 2022 7:14 PM > To: Quinn Tran <qutran@marvell.com>; Nilesh Javali <njavali@marvell.com>; > GR-QLogic-Storage-Upstream <GR-QLogic-Storage- > Upstream@marvell.com>; James E.J. Bottomley <jejb@linux.ibm.com>; > Martin K. Petersen <martin.petersen@oracle.com> > Cc: linux-scsi@vger.kernel.org > Subject: [EXT] [PATCH] scsi: qla2xxx: Disable ATIO interrupt coalesce for quad > port ISP27XX > > External Email > > ---------------------------------------------------------------------- > This message was previously sent to scst-devel: > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__sourceforge.net_p_scst_mailman_scst-2Ddevel_thread_d381eb86- > 2Dd446-2D4f9e-2D03c3-2Daa93d93dd074-2540cybernetics.com_- > 23msg37682919&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FAW9wuzbtH > IZL7SV63sr8rG59Hctu-eGu0G9pxwOXgQ&m=5GSS_PnOk-k6a- > MRuYO0WiUj5AYxgz6x5377udNiDMopJa4pfZuAGdw9D6_7hGm0&s=Zr- > 65TXp2V-zJGp7xf3uPA_nJIDQdeTeJ11VpYsiBKc&e= > > I am using a quad-port 16 Gbps QLE2694L (ISP2071) with SCST 3.6 + > qla2x00t-32gbit and kernel 5.15. The following old commit that went > into upstream kernel v4.16 is causing a problem for me: > > commit d2b292c3f6fdef5819a276acd64915bae6384a7f > Author: Quinn Tran <quinn.tran@cavium.com> > Date: Thu Dec 28 12:33:17 2017 -0800 > > scsi: qla2xxx: Enable ATIO interrupt handshake for ISP27XX > > Enable ATIO Q interrupt handshake for ISP27XX. This patch > coalesce ATIO's interrupts for Quad port ISP27XX adapter. > Interrupt coalesce allows performance to scale for this > specific case. > > This commit was present in qla2xxx when qla2x00t-32gbit was first > imported into SCST. > > With the "ATIO interrupt coalesce" enabled by the above patch, ATIO > interrupts are handled by qla24xx_msix_default() rather than by > qla83xx_msix_atio_q() for the ISP2071. I guess this is supposed to > generate fewer interrupts so that ATIO entries can be handled in larger > batches. But this causes a problem where sometimes > qlt_24xx_process_atio_queue() returns while the hardware is still adding > ATIO entries to the queue, but then the hardware doesn't generate > another interrupt until a separate event sometimes minutes later. This > leaves incoming ATIO entries (i.e. incoming target-mode SCSI commands) > ignored in the adapter's hardware queue for a long time until the host > sends another command at a different time to generate an interrupt. > There does not seem to be any timeout function that generates an > interrupt after a short period of inactivity to process the remainder of > the ATIO queue, which is what would be necessary for interrupt coalesce > to function properly. > > In my case I am using virtual tape drives, which generally process only > one command at a time, so there will often never be a "next" command to > generate an interrupt until the previous command completes. If the > previous command is stuck in the adapter's ATIO queue, then the host > will timeout and abort the command, and the task management function to > abort the command generates the interrupt that causes the command to > finally be received by SCST, so the command appears to have been aborted > immediately. With disk drives the situation might be a bit different > since a busy disk might get a steady stream of commands to generate > additional interrupts to process the ATIO queue, but even that is not > guaranteed, so there might still be problems. > > I tried to fix the problem by modifying qla24xx_msix_default() to call > qlt_24xx_process_atio_queue() before wrt_reg_dword(®->hccr, > HCCRX_CLR_RISC_INT) instead of after, but that didn't help. The problem > was solved by disabling the ATIO interrupt coalesce feature and > re-enabling the dedicated ATIO MSI-X interrupt (qla83xx_msix_atio_q()) > for calling qlt_24xx_process_atio_queue(). Another workaround is to use > ql2xenablemsix=0, but of course keeping MSI-X enabled is better. > > The patch below is against the upstream Linux kernel. It can be applied > to SCST with "patch -p4" from the qla2x00t-32gbit directory. > > If someone really wants to keep the interrupt coalesce feature, it could > be turned into a module parameter instead. > > From e5888ce5dbffff2d38d9dafd4841d6d3bcda4365 Mon Sep 17 00:00:00 > 2001 > From: Tony Battersby <tonyb@cybernetics.com> > Date: Thu, 7 Jul 2022 15:08:01 -0400 > Subject: [PATCH] scsi: qla2xxx: Disable ATIO interrupt coalesce for quad port > ISP27XX > > This partially reverts commit d2b292c3f6fdef5819a276acd64915bae6384a7f. > > For some workloads where the host sends a batch of commands and then > pauses, ATIO interrupt coalesce can cause some incoming ATIO entries to > be ignored for extended periods of time, resulting in slow performance, > timeouts, and aborted commands. So disable interrupt coalesce and > re-enable the dedicated ATIO MSI-X interrupt. > > Signed-off-by: Tony Battersby <tonyb@cybernetics.com> > --- > drivers/scsi/qla2xxx/qla_target.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_target.c > b/drivers/scsi/qla2xxx/qla_target.c > index cb97f625970d..80fd9980dfc9 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -6932,14 +6932,8 @@ qlt_24xx_config_rings(struct scsi_qla_host *vha) > > if (ha->flags.msix_enabled) { > if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) { > - if (IS_QLA2071(ha)) { > - /* 4 ports Baker: Enable Interrupt Handshake > */ > - icb->msix_atio = 0; > - icb->firmware_options_2 |= > cpu_to_le32(BIT_26); > - } else { > - icb->msix_atio = cpu_to_le16(msix->entry); > - icb->firmware_options_2 &= > cpu_to_le32(~BIT_26); > - } > + icb->msix_atio = cpu_to_le16(msix->entry); > + icb->firmware_options_2 &= cpu_to_le32(~BIT_26); > ql_dbg(ql_dbg_init, vha, 0xf072, > "Registering ICB vector 0x%x for atio que.\n", > msix->entry); Thanks for the patch. Reviewed-by: Nilesh Javali <njavali@marvell.com>
Tony, > This partially reverts commit d2b292c3f6fdef5819a276acd64915bae6384a7f. > > For some workloads where the host sends a batch of commands and then > pauses, ATIO interrupt coalesce can cause some incoming ATIO entries > to be ignored for extended periods of time, resulting in slow > performance, timeouts, and aborted commands. So disable interrupt > coalesce and re-enable the dedicated ATIO MSI-X interrupt. Applied to 5.20/scsi-staging, thanks!
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index cb97f625970d..80fd9980dfc9 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -6932,14 +6932,8 @@ qlt_24xx_config_rings(struct scsi_qla_host *vha) if (ha->flags.msix_enabled) { if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) { - if (IS_QLA2071(ha)) { - /* 4 ports Baker: Enable Interrupt Handshake */ - icb->msix_atio = 0; - icb->firmware_options_2 |= cpu_to_le32(BIT_26); - } else { - icb->msix_atio = cpu_to_le16(msix->entry); - icb->firmware_options_2 &= cpu_to_le32(~BIT_26); - } + icb->msix_atio = cpu_to_le16(msix->entry); + icb->firmware_options_2 &= cpu_to_le32(~BIT_26); ql_dbg(ql_dbg_init, vha, 0xf072, "Registering ICB vector 0x%x for atio que.\n", msix->entry);