The MHI interrupt handling issue

The MHI interrupt handling issue

[Loic Poulain]

Hi folks,

Before putting my hands into that, I wanted to expose a MHI interrupt
problem, comments are welcome.

Currently, the hard IRQ handler for event rings do nothing except
triggering a tasklet as bottom half that in turn will take care of
retrieving buffer(s). That leads to have an unexpected high amount of
interrupts when I perform throughput testing with mhi-net (though it
applies for any mhi client driver).

The point is that usually, an hard interrupt handler is responsible
for triggering the bottom half handler but also for
clearing/suspending interrupt on device side. However, AFAIK, there is
no such possibility in the MHI protocol. Since the interrupt is not
handled in the hard irq handler, it is triggered again once interrupt
are re-enabled, and even during the tasklet execution... at the end,
that makes a lot of unnecessary interrupts, that introduce latency and
participate to system load...

I added some printk to highlight that issue:
[11564.689202] mhi_irq_handler 55d32b8
[11564.689485] mhi_irq_handler 55d32b8
[11564.690011] mhi_irq_handler 55d32b8
[11564.690397] [55d32b8] mhi_process_data_event_ring start
[11564.690667] mhi_irq_handler 55d32b8
[11564.690937] mhi_irq_handler 55d32b8
[11564.691207] mhi_irq_handler 55d32b8
[11564.691475] mhi_irq_handler 55d32b8
[11564.692076] [55d32b8] mhi_process_data_event_ring done
[...]

I see two solutions to fix that problem:
- Manage events directly in the hard-irq handler (no more tasklet)
- Use threaded IRQ with IRQF_ONESHOT flag, to keep interrupt masked
until threaded handler has completed.

Regards,
Loic

Re: The MHI interrupt handling issue

[Jeffrey Hugo]

On 12/7/2020 6:20 AM, Loic Poulain wrote:
> Hi folks,
> 
> Before putting my hands into that, I wanted to expose a MHI interrupt
> problem, comments are welcome.
> 
> Currently, the hard IRQ handler for event rings do nothing except
> triggering a tasklet as bottom half that in turn will take care of
> retrieving buffer(s). That leads to have an unexpected high amount of
> interrupts when I perform throughput testing with mhi-net (though it
> applies for any mhi client driver).
> 
> The point is that usually, an hard interrupt handler is responsible
> for triggering the bottom half handler but also for
> clearing/suspending interrupt on device side. However, AFAIK, there is
> no such possibility in the MHI protocol. Since the interrupt is not

I think Linux side improvements would always be welcome, but have you 
looked at BEI and INTMODT?  They are features defined in the MHI spec 
for addressing interrupt storms.


> handled in the hard irq handler, it is triggered again once interrupt
> are re-enabled, and even during the tasklet execution... at the end,
> that makes a lot of unnecessary interrupts, that introduce latency and
> participate to system load...
> 
> I added some printk to highlight that issue:
> [11564.689202] mhi_irq_handler 55d32b8
> [11564.689485] mhi_irq_handler 55d32b8
> [11564.690011] mhi_irq_handler 55d32b8
> [11564.690397] [55d32b8] mhi_process_data_event_ring start
> [11564.690667] mhi_irq_handler 55d32b8
> [11564.690937] mhi_irq_handler 55d32b8
> [11564.691207] mhi_irq_handler 55d32b8
> [11564.691475] mhi_irq_handler 55d32b8
> [11564.692076] [55d32b8] mhi_process_data_event_ring done
> [...]
> 
> I see two solutions to fix that problem:
> - Manage events directly in the hard-irq handler (no more tasklet)
> - Use threaded IRQ with IRQF_ONESHOT flag, to keep interrupt masked
> until threaded handler has completed.
> 
> Regards,
> Loic
> 


-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: The MHI interrupt handling issue

[Loic Poulain]

Hi Jeffrey,

On Mon, 7 Dec 2020 at 16:48, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>
> On 12/7/2020 6:20 AM, Loic Poulain wrote:
> > Hi folks,
> >
> > Before putting my hands into that, I wanted to expose a MHI interrupt
> > problem, comments are welcome.
> >
> > Currently, the hard IRQ handler for event rings do nothing except
> > triggering a tasklet as bottom half that in turn will take care of
> > retrieving buffer(s). That leads to have an unexpected high amount of
> > interrupts when I perform throughput testing with mhi-net (though it
> > applies for any mhi client driver).
> >
> > The point is that usually, an hard interrupt handler is responsible
> > for triggering the bottom half handler but also for
> > clearing/suspending interrupt on device side. However, AFAIK, there is
> > no such possibility in the MHI protocol. Since the interrupt is not
>
> I think Linux side improvements would always be welcome, but have you
> looked at BEI and INTMODT?  They are features defined in the MHI spec
> for addressing interrupt storms.

Yes, that interrupt coalescing feature is working well, and I use it
in that context. But Actually the problem is not really that the
device generates too many interrupts, but that the driver does not
mask the interrupt while handling it, causing replicated interrupts,
and so useless hard interrupt handler executions. I need to
investigate how bad it impact the system/transfers though.


>
>
> > handled in the hard irq handler, it is triggered again once interrupt
> > are re-enabled, and even during the tasklet execution... at the end,
> > that makes a lot of unnecessary interrupts, that introduce latency and
> > participate to system load...
> >
> > I added some printk to highlight that issue:
> > [11564.689202] mhi_irq_handler 55d32b8
> > [11564.689485] mhi_irq_handler 55d32b8
> > [11564.690011] mhi_irq_handler 55d32b8
> > [11564.690397] [55d32b8] mhi_process_data_event_ring start
> > [11564.690667] mhi_irq_handler 55d32b8
> > [11564.690937] mhi_irq_handler 55d32b8
> > [11564.691207] mhi_irq_handler 55d32b8
> > [11564.691475] mhi_irq_handler 55d32b8
> > [11564.692076] [55d32b8] mhi_process_data_event_ring done
> > [...]
> >
> > I see two solutions to fix that problem:
> > - Manage events directly in the hard-irq handler (no more tasklet)
> > - Use threaded IRQ with IRQF_ONESHOT flag, to keep interrupt masked
> > until threaded handler has completed.
> >
> > Regards,
> > Loic

Re: The MHI interrupt handling issue

[Bhaumik Bhatt]

On 2020-12-07 09:02 AM, Loic Poulain wrote:
> Hi Jeffrey,
> 
> On Mon, 7 Dec 2020 at 16:48, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>> 
>> On 12/7/2020 6:20 AM, Loic Poulain wrote:
>> > Hi folks,
>> >
>> > Before putting my hands into that, I wanted to expose a MHI interrupt
>> > problem, comments are welcome.
>> >
>> > Currently, the hard IRQ handler for event rings do nothing except
>> > triggering a tasklet as bottom half that in turn will take care of
>> > retrieving buffer(s). That leads to have an unexpected high amount of
>> > interrupts when I perform throughput testing with mhi-net (though it
>> > applies for any mhi client driver).
>> >
>> > The point is that usually, an hard interrupt handler is responsible
>> > for triggering the bottom half handler but also for
>> > clearing/suspending interrupt on device side. However, AFAIK, there is
>> > no such possibility in the MHI protocol. Since the interrupt is not
>> 
>> I think Linux side improvements would always be welcome, but have you
>> looked at BEI and INTMODT?  They are features defined in the MHI spec
>> for addressing interrupt storms.
> 
> Yes, that interrupt coalescing feature is working well, and I use it
> in that context. But Actually the problem is not really that the
> device generates too many interrupts, but that the driver does not
> mask the interrupt while handling it, causing replicated interrupts,
> and so useless hard interrupt handler executions. I need to
> investigate how bad it impact the system/transfers though.
> 
> 
>> 
>> 
>> > handled in the hard irq handler, it is triggered again once interrupt
>> > are re-enabled, and even during the tasklet execution... at the end,
>> > that makes a lot of unnecessary interrupts, that introduce latency and
>> > participate to system load...
>> >
>> > I added some printk to highlight that issue:
>> > [11564.689202] mhi_irq_handler 55d32b8
>> > [11564.689485] mhi_irq_handler 55d32b8
>> > [11564.690011] mhi_irq_handler 55d32b8
>> > [11564.690397] [55d32b8] mhi_process_data_event_ring start
>> > [11564.690667] mhi_irq_handler 55d32b8
>> > [11564.690937] mhi_irq_handler 55d32b8
>> > [11564.691207] mhi_irq_handler 55d32b8
>> > [11564.691475] mhi_irq_handler 55d32b8
>> > [11564.692076] [55d32b8] mhi_process_data_event_ring done
>> > [...]
>> >
>> > I see two solutions to fix that problem:
>> > - Manage events directly in the hard-irq handler (no more tasklet)
>> > - Use threaded IRQ with IRQF_ONESHOT flag, to keep interrupt masked
>> > until threaded handler has completed.
>> >
>> > Regards,
>> > Loic

I remember from an earlier conversation with Jeff and Mani that 
IRQF_ONESHOT
cannot be used when a IRQF_SHARED flag is also present when we have a 
single
shared IRQ line.

Jeff has this use case where BHI intvec and rest of the MHI event rings 
are
sharing the same IRQ line.

We have been using napi_poll() with 5msec INTMODT for high TPUT data 
transfers.

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