linux/drivers
Ingo Molnar 3a10ccebe9 [PATCH] lock validator: fix ns83820.c irq-flags bug
Barry K. Nathan reported the following lockdep warning:

[  197.343948] BUG: warning at kernel/lockdep.c:1856/trace_hardirqs_on()
[  197.345928]  [<c010329b>] show_trace_log_lvl+0x5b/0x105
[  197.346359]  [<c0103896>] show_trace+0x1b/0x20
[  197.346759]  [<c01038ed>] dump_stack+0x1f/0x24
[  197.347159]  [<c012efa2>] trace_hardirqs_on+0xfb/0x185
[  197.348873]  [<c029b009>] _spin_unlock_irq+0x24/0x2d
[  197.350620]  [<e09034e8>] do_tx_done+0x171/0x179 [ns83820]
[  197.350895]  [<e090445c>] ns83820_irq+0x149/0x20b [ns83820]
[  197.351166]  [<c013b4b8>] handle_IRQ_event+0x1d/0x52
[  197.353216]  [<c013c6c2>] handle_level_irq+0x97/0xe1
[  197.355157]  [<c01048c3>] do_IRQ+0x8b/0xac
[  197.355612]  [<c0102d9d>] common_interrupt+0x25/0x2c

this is caused because the ns83820 driver re-enables irq flags
in hardirq context.

While legal in theory, in practice it should only be done if the
hardware is really old and has some very high overhead in its ISR.
(such as PIO IDE)

For modern hardware, running ISRs with irqs enabled is discouraged,
because 1) new hardware is fast enough to not cause latency problems
2) allowing the nesting of hardware interrupts only 'spreads out'
the handling of the current ISR, causing extra cachemisses that would
otherwise not happen. Furthermore, on architectures where ISRs share
the kernel stacks, enabling interrupts in ISRs introduces a much
higher kernel-stack-nesting and thus kernel-stack-overflow risk.
3) not managing irq-flags via the _irqsave / _irqrestore variants
is dangerous: it's easy to forget whether one function nests inside
another, and irq flags might be mismanaged.

In the few cases where re-enabling interrupts in an ISR is considered
useful (and unavoidable), it has to be taught to the lock validator
explicitly (because the lock validator needs the "no ISR ever enables
hardirqs" artificial simplification to keep the IRQ/softirq locking
dependencies manageable).

This teaching is done via the explicit use local_irq_enable_in_hardirq().
On a stock kernel this maps to local_irq_enable(). If the lock validator
is enabled then this does not enable interrupts.

Now, the analysis of drivers/net/ns83820.c's irq flags use: the
irq-enabling in irq context seems intentional, but i dont think it's
justified. Furthermore, the driver suffers from problem #3 above too,
in ns83820_tx_timeout() it disables irqs via local_irq_save(), but
then it calls do_tx_done() which does a spin_unlock_irq(),
re-enabling for a function that does not expect it! While currently
this bug seems harmless (only some debug printout seems to be
affected by it), it's nevertheless something to be fixed.

So this patch makes the ns83820 ISR irq-flags-safe, and cleans up
do_tx_done() use and locking to avoid the ns83820_tx_timeout() bug.

From: Arjan van de Ven <arjan@linux.intel.com>

  ns83820_mib_isr takes the misc_lock in IRQ context.  All other places that
  do this in the ISR already use _irqsave versions, make this consistent at
  least.  At some point in the future someone should audit the driver to see
  if all _irqsave's in the ISR can go away, this is generally an iffy/fragile
  proposition though; for now get it safe, simple and consistent.

From: Arjan van de Ven <arjan@linux.intel.com>

ok this is a real driver deadlock:

The ns83820 driver enabled interrupts (by unlocking the misc_lock with
_irq) while still holding the rx_info.lock, which is required to be irq
safe since it's used in the ISR like this:
                writel(1, dev->base + IER);
                spin_unlock_irq(&dev->misc_lock);
                kick_rx(ndev);
                spin_unlock_irq(&dev->rx_info.lock);

This is can cause a deadlock if an irq was pending at the first
spin_unlock_irq already, or if one would hit during kick_rx().
Simply remove the first _irq solves this

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
2006-07-05 14:08:08 -04:00
..
acorn [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
acpi Merge branch 'release' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6 2006-07-03 21:32:50 -07:00
amba [PATCH] 64bit resource: fix up printks for resources in misc drivers 2006-06-27 09:23:59 -07:00
atm [ATM]: add+use poison defines 2006-07-03 19:48:25 -07:00
base Merge git://git.kernel.org/pub/scm/linux/kernel/git/bunk/trivial 2006-06-30 15:39:30 -07:00
block Merge git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc 2006-07-03 15:28:34 -07:00
bluetooth [Bluetooth] Add platform device for virtual and serial devices 2006-07-03 19:54:00 -07:00
cdrom [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
char Merge git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc 2006-07-03 15:28:34 -07:00
clocksource [PATCH] GTOD: add scx200 HRT clocksource 2006-06-26 09:58:21 -07:00
connector [PATCH] connector-exports 2006-06-23 07:43:06 -07:00
cpufreq Merge master.kernel.org:/pub/scm/linux/kernel/git/davej/cpufreq 2006-07-04 14:00:26 -07:00
crypto [CRYPTO] padlock: Rearrange context structure to reduce code size 2006-06-26 17:34:39 +10:00
dio
dma [IOAT]: fix kernel-doc in source files 2006-07-03 19:45:31 -07:00
edac [PATCH] EDAC: fix module names quoted in sysfs 2006-07-01 09:55:58 -07:00
eisa Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
fc4 [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
firmware Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
hwmon Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
i2c [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
ide [PATCH] lockdep: annotate on-stack completions 2006-07-03 15:27:09 -07:00
ieee1394 [PATCH] lockdep: annotate ieee1394 skb-queue-head locking 2006-07-03 15:27:08 -07:00
infiniband [PATCH] mthca: initialize send and receive queue locks separately 2006-07-04 10:24:57 -07:00
input [PATCH] lockdep: annotate serio 2006-07-03 15:27:07 -07:00
isdn [PATCH] irq-flags: isdn: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
leds Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
macintosh [POWERPC] Add new interrupt mapping core and change platforms to use it 2006-07-03 21:36:01 +10:00
mca
md [PATCH] lockdep: annotate blkdev nesting 2006-07-03 15:27:10 -07:00
media [PATCH] irq-flags: media: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
message Merge ../scsi-misc-2.6 2006-07-03 09:41:12 -05:00
mfd [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
misc [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
mmc [PATCH] lockdep: annotate on-stack completions, mmc 2006-07-03 15:27:10 -07:00
mtd Merge git://git.infradead.org/mtd-2.6 2006-07-03 21:29:08 -07:00
net [PATCH] lock validator: fix ns83820.c irq-flags bug 2006-07-05 14:08:08 -04:00
nubus Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
oprofile [PATCH] oprofile: convert from semaphores to mutexes 2006-06-25 10:01:04 -07:00
parisc [PATCH] irq-flags: PARISC: Use the new IRQF_ constants 2006-07-02 13:58:52 -07:00
parport [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
pci [PATCH] irq-flags: pci: Use the new IRQF_ constants 2006-07-02 13:58:52 -07:00
pcmcia Merge branch 'genirq' of master.kernel.org:/home/rmk/linux-2.6-arm 2006-07-02 15:07:45 -07:00
pnp [PATCH] PNPACPI: support shareable interrupts 2006-07-03 15:26:58 -07:00
rapidio Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
rtc [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
s390 [PATCH] zfcp: fix incorrect usage of fsf_req_list_lock 2006-07-03 21:25:20 -07:00
sbus [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
scsi Merge master.kernel.org:/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6 2006-07-03 21:27:18 -07:00
serial Merge branch 'devel' of master.kernel.org:/home/rmk/linux-2.6-serial 2006-07-03 21:31:05 -07:00
sh
sn [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
spi [PATCH] lockdep: annotate on-stack completions 2006-07-03 15:27:09 -07:00
tc [PATCH] irq-flags: misc drivers: Use the new IRQF_ constants 2006-07-02 13:58:50 -07:00
telephony Merge git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/devfs-2.6 2006-06-29 14:19:21 -07:00
usb Merge master.kernel.org:/home/rmk/linux-2.6-arm 2006-07-03 21:30:03 -07:00
video Merge git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc 2006-07-03 15:28:34 -07:00
w1 [PATCH] connector-exports 2006-06-23 07:43:06 -07:00
zorro Remove obsolete #include <linux/config.h> 2006-06-30 19:25:36 +02:00
Kconfig
Makefile [PATCH] Time: i386 Clocksource Drivers 2006-06-26 09:58:21 -07:00