Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240860 > unrolled thread

Re: [tip:irq/core] genirq: Handle force threading of irqs with primary and thread handler

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [tip:irq/core] genirq:  Handle force threading of irqs with primary and thread handler Felipe Balbi <balbi@ti.com> - 2015-10-06 21:10 +0200
    Re: [tip:irq/core] genirq: Handle force threading of irqs with  primary and thread handler Thomas Gleixner <tglx@linutronix.de> - 2015-10-09 12:40 +0200
      Re: [tip:irq/core] genirq: Handle force threading of irqs with primary and thread handler Felipe Balbi <balbi@ti.com> - 2015-10-09 16:10 +0200

#1240860 — Re: [tip:irq/core] genirq: Handle force threading of irqs with primary and thread handler

[Multipart message — attachments visible in raw view] — view raw

Hi Thomas,

tip-bot for Thomas Gleixner <tipbot@zytor.com> writes:
> Commit-ID:  2a1d3ab8986d1b2f598ffc42351d94166fa0f022
> Gitweb:     http://git.kernel.org/tip/2a1d3ab8986d1b2f598ffc42351d94166fa0f022
> Author:     Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Mon, 21 Sep 2015 11:01:10 +0200
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Tue, 22 Sep 2015 12:39:57 +0200
>
> genirq: Handle force threading of irqs with primary and thread handler
>
> Force threading of interrupts does not really deal with interrupts
> which are requested with a primary and a threaded handler. The current
> policy is to leave them alone and let the primary handler run in
> interrupt context, but we set the ONESHOT flag for those interrupts as
> well.
>
> Kohji Okuno debugged a problem with the SDHCI driver where the
> interrupt thread waits for a hardware interrupt to trigger, which can't
> work well because the hardware interrupt is masked due to the ONESHOT
> flag being set. He proposed to set the ONESHOT flag only if the
> interrupt does not provide a thread handler.
>
> Though that does not work either because these interrupts can be
> shared. So the other interrupt would rightfully get the ONESHOT flag
> set and therefor the same situation would happen again.
>
> To deal with this proper, we need to force thread the primary handler
> of such interrupts as well. That means that the primary interrupt
> handler is treated as any other primary interrupt handler which is not
> marked IRQF_NO_THREAD. The threaded handler becomes a separate thread
> so the SDHCI flow logic can be handled gracefully.
>
> The same issue was reported against 4.1-rt.
>
> Reported-and-tested-by: Kohji Okuno <okuno.kohji@jp.panasonic.com>
> Reported-By: Michal Smucr <msmucr@gmail.com>
> Reported-and-tested-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1509211058080.5606@nanos
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

this commit causes a performance regression for the USB driver on
several platforms (anybody using drivers/usb/dwc3, basically).

Here's the USB throughput with linux-next in 3 different scenarios:

1) Linux next without threadirqs cmdline

   test  0: sent     256.00 MB read      33.02 MB/s write      30.01 MB/s

2) Linux next with threadirqs on cmdline

   test  0: sent     256.00 MB read      30.70 MB/s write      27.89 MB/s

3) Linux next with threadirqs on cmdline + revert of $subject

   test  0: sent     256.00 MB read      32.93 MB/s write      29.85 MB/s


Considering this is trying to solve an issue found on the SDHCI driver,
shouldn't that be fixed instead ? Another option would be, of course, to
add IRQF_NO_THREAD to dwc3, but I'd like to avoid that if possible.

The way we try to use dwc3 is rather simple, actually. We use the
primary handle *only* to detect is $this device generated the IRQ and if
did we wake up the thread. We also don't make use of ONESHOT because we
mask $this device IRQs in the primary handler and only unmask after the
thread runs.

It's a bit surprising, to me at least, that simply running everything as
a thread would have such a measurable impact, but it does.

-- 
balbi

[toc] | [next] | [standalone]

#1243232 — Re: [tip:irq/core] genirq: Handle force threading of irqs with primary and thread handler

On Tue, 6 Oct 2015, Felipe Balbi wrote:
> this commit causes a performance regression for the USB driver on
> several platforms (anybody using drivers/usb/dwc3, basically).
> 
> Here's the USB throughput with linux-next in 3 different scenarios:
> 
> 1) Linux next without threadirqs cmdline
> 
>    test  0: sent     256.00 MB read      33.02 MB/s write      30.01 MB/s
> 
> 2) Linux next with threadirqs on cmdline
> 
>    test  0: sent     256.00 MB read      30.70 MB/s write      27.89 MB/s
> 
> 3) Linux next with threadirqs on cmdline + revert of $subject
> 
>    test  0: sent     256.00 MB read      32.93 MB/s write      29.85 MB/s
> 
> 
> Considering this is trying to solve an issue found on the SDHCI driver,
> shouldn't that be fixed instead ? Another option would be, of course, to
> add IRQF_NO_THREAD to dwc3, but I'd like to avoid that if possible.

It's not only an issue for SDHCI. It's a general problem with other
drivers as well.
 
> The way we try to use dwc3 is rather simple, actually. We use the
> primary handle *only* to detect is $this device generated the IRQ and if
> did we wake up the thread. We also don't make use of ONESHOT because we
> mask $this device IRQs in the primary handler and only unmask after the
> thread runs.

So in your case IRQF_NO_THREAD is really the solution. It will keep
your primary handler handled in the hard interrupt context. That will
work on RT as well.
 
> It's a bit surprising, to me at least, that simply running everything as
> a thread would have such a measurable impact, but it does.

I'm surprised of the size of the impact as well. I wouldn't have
expected that another kernel thread context switch makes such a
difference.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243401 — Re: [tip:irq/core] genirq: Handle force threading of irqs with primary and thread handler

[Multipart message — attachments visible in raw view] — view raw

Hi,

Thomas Gleixner <tglx@linutronix.de> writes:
> On Tue, 6 Oct 2015, Felipe Balbi wrote:
>> this commit causes a performance regression for the USB driver on
>> several platforms (anybody using drivers/usb/dwc3, basically).
>> 
>> Here's the USB throughput with linux-next in 3 different scenarios:
>> 
>> 1) Linux next without threadirqs cmdline
>> 
>>    test  0: sent     256.00 MB read      33.02 MB/s write      30.01 MB/s
>> 
>> 2) Linux next with threadirqs on cmdline
>> 
>>    test  0: sent     256.00 MB read      30.70 MB/s write      27.89 MB/s
>> 
>> 3) Linux next with threadirqs on cmdline + revert of $subject
>> 
>>    test  0: sent     256.00 MB read      32.93 MB/s write      29.85 MB/s
>> 
>> 
>> Considering this is trying to solve an issue found on the SDHCI driver,
>> shouldn't that be fixed instead ? Another option would be, of course, to
>> add IRQF_NO_THREAD to dwc3, but I'd like to avoid that if possible.
>
> It's not only an issue for SDHCI. It's a general problem with other
> drivers as well.
>  
>> The way we try to use dwc3 is rather simple, actually. We use the
>> primary handle *only* to detect is $this device generated the IRQ and if
>> did we wake up the thread. We also don't make use of ONESHOT because we
>> mask $this device IRQs in the primary handler and only unmask after the
>> thread runs.
>
> So in your case IRQF_NO_THREAD is really the solution. It will keep
> your primary handler handled in the hard interrupt context. That will
> work on RT as well.

all right. I'll patch that up. Thanks

-- 
balbi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web