archive mirror
 help / color / mirror / Atom feed
From: Jeroen Van den Keybus <>
To: Edward Donovan <>
Cc: Linus Torvalds <>,
	Chris Palmer <>,
	Robert Hancock <>,
	Andrew Morton <>,
	Len Brown <>,,,,,,,
	Thomas Gleixner <>, Ingo Molnar <>
Subject: Re: ASM1083 PCIx-PCI bridge interrupts - widespread problems
Date: Thu, 2 Feb 2012 23:41:44 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

>> And we'd probably need to limit the warning messages if we start
>> re-enabling it - so that people with constantly screaming interrupts
>> don't get a constant stream of 10 "nobody cared, disabling" messages
>> per second.

Main reason for the patch to comment out __report_bad_irq(). There's
other printk's in there right now, allowing monitoring of the patch's
(hack's) performance. After 2 months of testing, the following is a
typical result on my Asus E45M1-M board:

[1675739.482843] Disabling IRQ 16
[1675739.488056] Polling IRQ 16
[1675740.288244] Reenabling IRQ 16
[1675740.288363] Disabling IRQ 16
[1675740.296132] Polling IRQ 16
[1675802.512233] Polling IRQ 16
[1675803.312244] Reenabling IRQ 16
[1675803.312362] Disabling IRQ 16
[1675803.320233] Polling IRQ 16
[1675804.120229] Reenabling IRQ 16

Because this particular IRQ is only asserted once every 64 s, polling
mode stays active for that amount of time, until a new INTx Deassert
is received. So it appears that INTx Deassert is not delayed, but
simply lost (either not sent or not received by the IO-APIC).

I did a test ( in which I
programmed an e1000 to issue one of the interrupts it doesn't use/need
(RXT0). From that log, it is clear that raising and clearing the IRQ
after more than 60 µs did not generate the expected INTx Deassert
either. There is serious trouble with this device.

> * New logic in the generic IRQ code, in spurious.c, adding a "try
> polling, then re-enable for
>  a while" method, for everybody?

Something like it (poll for a while, then try reenabling). Please see
the patch ( for the general idea.

> * Could there be more hardware-specifc code, to crank up the
> frequency, when you do have this chip?  I don't think we have this
> facility at present: would we let the arch-or-drivers code set a
> variable, to be honored by irq/spurious.c?

I would propose to crank up the frequency adaptively. Whenever
reenabling fails (i.e. is followed by a storm immediately), the
poll_spurious_irq_timer interval may be increased gradually up to,
say, 100ms (the value currently used by irqpoll). In the other cases,
it could be decreased progressively to e.g. 1 ms. So add or subtract
one ms of polling time whenever reenabling fails or succeeds,
respectively. Attempt to reenable an IRQ could occur after a fixed
amount of polling cycles (100).

Alternatively, the interval could also be modified by the number of
interrupts received in an interval.

- When, Heaven forbid, more than one IRQ is handled by this mechanism,
what would the polling interval need to be ? The shortest of them all
? Can/should timers be created dynamically ?
- Would struct irq_desc be an appropriate place to keep the per-irq
variables to accomplish all this ? I noticed that there is also
- Are there any alignment/security requirements on struct irq_desc to
be aware of ?
- Alan Cox also suggested that IRQ 0 'magic' could be an issue. I
cannot really find what he refers to in spurious.c, but it may be
important ?
- Are there any drivers that would not be able to operate in a polling fashion ?


  parent reply	other threads:[~2012-02-02 22:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
     [not found] ` <>
2012-01-30 15:04   ` ASM1083 PCIx-PCI bridge interrupts - widespread problems Chris Palmer
2012-01-31  2:12     ` Robert Hancock
2012-01-31 12:08       ` Chris Palmer
2012-02-02 19:20         ` Edward Donovan
2012-02-02 19:28           ` Linus Torvalds
2012-02-02 20:22             ` Edward Donovan
2012-02-02 21:39               ` Clemens Ladisch
2012-02-02 22:41               ` Jeroen Van den Keybus [this message]
2012-02-03  1:59                 ` Edward Donovan
2012-02-03  8:51                   ` Müller Keve

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).