2015-11-09 01:58:46

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH V2 3/3] scsi: mptxsas: offload IRQ execution

The mpt2sas and mpt3sas drivers are spinning forever in
their IRQ handlers if there are a lot of jobs queued up
by the PCIe card. This handler is causing spikes for
the rest of the system and sluggish behavior.

Marking all MSI interrupts as non-shared and moving the
MSI interrupts to thread context. This relexes the rest
of the system execution.

Signed-off-by: Sinan Kaya <[email protected]>
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 12 ++++++++----
drivers/scsi/mpt3sas/mpt3sas_base.c | 13 +++++++++----
2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index c61c82a..b619a0e 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1359,14 +1359,18 @@ _base_request_irq(struct MPT2SAS_ADAPTER *ioc, u8 index, u32 vector)
cpumask_clear(reply_q->affinity_hint);

atomic_set(&reply_q->busy, 0);
- if (ioc->msix_enable)
+ if (ioc->msix_enable) {
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
MPT2SAS_DRIVER_NAME, ioc->id, index);
- else
+ r = request_threaded_irq(vector, NULL, _base_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ reply_q->name, reply_q);
+ } else {
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
MPT2SAS_DRIVER_NAME, ioc->id);
- r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
- reply_q);
+ r = request_irq(vector, _base_interrupt, IRQF_SHARED,
+ reply_q->name, reply_q);
+ }
if (r) {
printk(MPT2SAS_ERR_FMT "unable to allocate interrupt %d!\n",
reply_q->name, vector);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 6dc391c..62bee43 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1661,14 +1661,19 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
cpumask_clear(reply_q->affinity_hint);

atomic_set(&reply_q->busy, 0);
- if (ioc->msix_enable)
+ if (ioc->msix_enable) {
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
MPT3SAS_DRIVER_NAME, ioc->id, index);
- else
+
+ r = request_threaded_irq(vector, NULL, _base_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ reply_q->name, reply_q);
+ } else {
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
MPT3SAS_DRIVER_NAME, ioc->id);
- r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
- reply_q);
+ r = request_irq(vector, _base_interrupt, IRQF_SHARED,
+ reply_q->name, reply_q);
+ }
if (r) {
pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
reply_q->name, vector);
--
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project


2015-11-09 07:15:49

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH V2 3/3] scsi: mptxsas: offload IRQ execution

On 11/09/2015 02:57 AM, Sinan Kaya wrote:
> The mpt2sas and mpt3sas drivers are spinning forever in
> their IRQ handlers if there are a lot of jobs queued up
> by the PCIe card. This handler is causing spikes for
> the rest of the system and sluggish behavior.
>
> Marking all MSI interrupts as non-shared and moving the
> MSI interrupts to thread context. This relexes the rest
> of the system execution.
>
NACK.

If there is a scalability issue when handling interrupts
it should be fixed in the driver directly.

Looking at the driver is should be possible to implement
a worker thread handling the reply descriptor, and having the
interrupt only to fetch the reply descriptor.

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

2015-11-09 14:01:07

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH V2 3/3] scsi: mptxsas: offload IRQ execution



On 11/9/2015 2:15 AM, Hannes Reinecke wrote:
> On 11/09/2015 02:57 AM, Sinan Kaya wrote:
>> The mpt2sas and mpt3sas drivers are spinning forever in
>> their IRQ handlers if there are a lot of jobs queued up
>> by the PCIe card. This handler is causing spikes for
>> the rest of the system and sluggish behavior.
>>
>> Marking all MSI interrupts as non-shared and moving the
>> MSI interrupts to thread context. This relexes the rest
>> of the system execution.
>>
> NACK.
>
> If there is a scalability issue when handling interrupts
> it should be fixed in the driver directly.
>
> Looking at the driver is should be possible to implement
> a worker thread handling the reply descriptor, and having the
> interrupt only to fetch the reply descriptor.

I'll take a look.

>
> Cheers,
>
> Hannes
>

--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

2015-11-10 06:00:01

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH V2 3/3] scsi: mptxsas: offload IRQ execution



On 11/9/2015 2:15 AM, Hannes Reinecke wrote:
> On 11/09/2015 02:57 AM, Sinan Kaya wrote:
>> The mpt2sas and mpt3sas drivers are spinning forever in
>> their IRQ handlers if there are a lot of jobs queued up
>> by the PCIe card. This handler is causing spikes for
>> the rest of the system and sluggish behavior.
>>
>> Marking all MSI interrupts as non-shared and moving the
>> MSI interrupts to thread context. This relexes the rest
>> of the system execution.
>>
> NACK.
>
> If there is a scalability issue when handling interrupts
> it should be fixed in the driver directly.
>
> Looking at the driver is should be possible to implement
> a worker thread handling the reply descriptor, and having the
> interrupt only to fetch the reply descriptor.
>

Can you go into the detail about which part of the _base_interrupt
function needs to be executed in ISR context and which part can be
queued up to worker thread?

I'm not familiar with the hardware or the code. That's why, I moved the
entire ISR into the thread context.



> Cheers,
>
> Hannes
>

--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

2016-03-16 15:31:14

by Christopher Covington

[permalink] [raw]
Subject: Re: [PATCH V2 3/3] scsi: mptxsas: offload IRQ execution

On 11/10/2015 12:59 AM, Sinan Kaya wrote:
>
> On 11/9/2015 2:15 AM, Hannes Reinecke wrote:
>> On 11/09/2015 02:57 AM, Sinan Kaya wrote:
>>> The mpt2sas and mpt3sas drivers are spinning forever in
>>> their IRQ handlers if there are a lot of jobs queued up
>>> by the PCIe card. This handler is causing spikes for
>>> the rest of the system and sluggish behavior.
>>>
>>> Marking all MSI interrupts as non-shared and moving the
>>> MSI interrupts to thread context. This relexes the rest
>>> of the system execution.
>>>
>> NACK.
>>
>> If there is a scalability issue when handling interrupts
>> it should be fixed in the driver directly.
>>
>> Looking at the driver is should be possible to implement
>> a worker thread handling the reply descriptor, and having the
>> interrupt only to fetch the reply descriptor.
>>
>
> Can you go into the detail about which part of the _base_interrupt
> function needs to be executed in ISR context and which part can be
> queued up to worker thread?
>
> I'm not familiar with the hardware or the code. That's why, I moved the
> entire ISR into the thread context.

Hannes or others, any suggestions?

Thanks,
Christopher Covington

--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project