Re: pull request: wireless 2011-11-22

From: Larry Finger
Date: Tue Nov 22 2011 - 16:31:03 EST


On 11/22/2011 03:13 PM, David Miller wrote:
From: David Miller<davem@xxxxxxxxxxxxx>
Date: Tue, 22 Nov 2011 16:05:11 -0500 (EST)

From: "John W. Linville"<linville@xxxxxxxxxxxxx>
Date: Tue, 22 Nov 2011 15:56:55 -0500

On Tue, Nov 22, 2011 at 03:14:29PM -0500, David Miller wrote:
From: "John W. Linville"<linville@xxxxxxxxxxxxx>
Date: Tue, 22 Nov 2011 14:35:05 -0500

Here is the latest batch of fixes intended for 3.2. This includes a
correction for a user-visible error in mac80211's debugfs info, a fix
for a potential memory corrupter in prism54, an endian fix for rt2x00,
an endian fix for mac80211, a fix for a NULL derefernce in cfg80211, a
locking fix and a deadlock fix for p54spi, and a pair of rt2x00 fixes
for handling some spurious interrupts that hardware can generate.

Please let me know if there are problems!

The rt2800pci change doesn't look correct.

If the IRQ line is shared with another device, this change will make it
never see interrupts. Once you say "IRQ_HANDLED" the IRQ dispatch
stops processing the interrupt handler list.

I thought this at first as well. But looking at the code in
kernel/irq/handle.c doesn't support that conclusion. In fact, every
handler gets invoked no matter what they all return. All of the irq
handler return values are ORed together and passed to note_interrupt.
Only if every irq handler returns IRQ_NONE does the code in
kernel/irq/spurious.c start getting involved.

Anyway, this seems to be safe even for shared interrupts. That said,
this is a bit ugly. But it makes a serious difference in performance
for those afflicted with this issue.

It just means that we won't notice spurious interrupts if the device
sharing the line with rt2800pci generates one.

This change is wrong.

BTW, look at it this way, if what you say is true John then what's the point
in returning any specific value at all?

Everyone can just return IRQ_HANDLED and everything would just work.

But you know that's not the case, and that it's important that this value
is returned accurately.

I was trying to find the thread that reported the improvement in performance with this change, but failed. Is it possible that their change just papered over an interrupt storm from some other device that shared that interrupt?

I'm following this because the rtlwifi-family of drivers already did what was suggested here. If this is wrong, then so is it.

Larry

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