Commit Graph

23 Commits

Author SHA1 Message Date
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
Jeff Garzik
9b91cf9daa [netdrvr] use dev_xxx() printk helpers, rather than dev_printk(KERN_xxx, ...
Suggested by Jiri Slaby.

Signed-off-by: Jeff Garzik <jeff@garzik.org>
2006-07-05 13:42:57 -04:00
Jeff Garzik
2e8a538d86 [netdrvr] Use dev_printk() when ethernet interface isn't available
For messages prior to register_netdev(), prefer dev_printk() because
that prints out both our driver name and our [PCI | whatever] bus id.

Updates: 8139{cp,too}, b44, bnx2, cassini, {eepro,epic}100, fealnx,
	 hamachi, ne2k-pci, ns83820, pci-skeleton, r8169.

Signed-off-by: Jeff Garzik <jeff@garzik.org>
2006-07-05 13:42:07 -04:00
Thomas Gleixner
1fb9df5d30 [PATCH] irq-flags: drivers/net: Use the new IRQF_ constants
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
2006-07-02 13:58:51 -07:00
Jörn Engel
6ab3d5624e Remove obsolete #include <linux/config.h>
Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
Signed-off-by: Adrian Bunk <bunk@stusta.de>
2006-06-30 19:25:36 +02:00
Eric Sesterhenn
5d9428de1a BUG_ON() Conversion in drivers/net/
this changes if() BUG(); constructs to BUG_ON() which is
cleaner, contains unlikely() and can better optimized away.

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
Signed-off-by: Adrian Bunk <bunk@stusta.de>
2006-04-02 13:52:48 +02:00
Matthias Gehre
910638ae7e [PATCH] Replace 0xff.. with correct DMA_xBIT_MASK
Replace all occurences of 0xff..  in calls to function pci_set_dma_mask()
and pci_set_consistant_dma_mask() with the corresponding DMA_xBIT_MASK from
linux/dma-mapping.h.

Signed-off-by: Matthias Gehre <M.Gehre@gmx.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
2006-03-28 09:16:07 -08:00
Jeff Garzik
46153552b4 Merge branch 'net-const' 2006-03-03 22:22:45 -05:00
Arjan van de Ven
f71e130966 Massive net driver const-ification. 2006-03-03 21:33:57 -05:00
Marcelo Feitoza Parisi
ff5688ae1c [PATCH] drivers/net/*: use time_after() and friends
They deal with wrapping correctly and are nicer to read.  Also make
jiffies-holding variables unsigned long.

Signed-off-by: Marcelo Feitoza Parisi <marcelo@feitoza.com.br>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
2006-01-17 07:59:23 -05:00
Arnaldo Carvalho de Melo
14c850212e [INET_SOCK]: Move struct inet_sock & helper functions to net/inet_sock.h
To help in reducing the number of include dependencies, several files were
touched as they were getting needed headers indirectly for stuff they use.

Thanks also to Alan Menegotto for pointing out that net/dccp/proto.c had
linux/dccp.h include twice.

Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2006-01-03 13:11:21 -08:00
Adrian Bunk
48888cc612 [PATCH] kill include/linux/eeprom.h
This patch kills include/linux/eeprom.h .

Rationale:
- it was only used by one single driver
- even this driver didn't do anything useful with it
- most of this file are non-inline and non-static functions (sic)

This removes include/linux/eeprom.h and cleans drivers/net/ns83820.c up.

If you think eeprom.h should be used more extensively, please consider:
- the code has to be moved from the header file to a .c file
- the currently empty write function has to be implemented
- ns83820.c or any other driver should actually use it

Noone did any of these during the more than 3 years eeprom.h already
exists...

Signed-off-by: Adrian Bunk <bunk@stusta.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
2005-11-07 21:50:00 -05:00
Jeff Garzik
59aee3c2a1 Merge branch 'master' 2005-10-13 21:22:27 -04:00
Al Viro
dd0fc66fb3 [PATCH] gfp flags annotations - part 1
- added typedef unsigned int __nocast gfp_t;

 - replaced __nocast uses for gfp flags with gfp_t - it gives exactly
   the same warnings as far as sparse is concerned, doesn't change
   generated code (from gcc point of view we replaced unsigned int with
   typedef) and documents what's going on far better.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
2005-10-08 15:00:57 -07:00
Jeff Garzik
d9e34325fd Merge branch 'upstream-fixes' 2005-10-04 05:30:02 -04:00
Randy Dunlap
81c5873227 [PATCH] ns83820: fix gfp flags type
Fix implicit nocast warnings in ns83820 code, including __nocast:
drivers/net/ns83820.c:603:46: warning: implicit cast to nocast type

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
2005-10-04 05:29:48 -04:00
Jeff Garzik
3c8c7b2f32 Merge branch 'upstream-fixes' 2005-10-03 22:06:19 -04:00
Randy Dunlap
0a1c80f111 [PATCH] ns83820: fix gfp flags type
Fix implicit nocast warnings in ns83820 code:
drivers/net/ns83820.c:603:46: warning: implicit cast to nocast type

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
2005-10-03 22:01:14 -04:00
Nishanth Aravamudan
3173c8907f [PATCH] drivers/net: fix-up schedule_timeout() usage
Use schedule_timeout_interruptible() instead of
set_current_state()/schedule_timeout() to reduce kernel size.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
2005-09-14 08:33:24 -04:00
David S. Miller
689be43945 [NET]: Remove gratuitous use of skb->tail in network drivers.
Many drivers use skb->tail unnecessarily.

In these situations, the code roughly looks like:

	dev = dev_alloc_skb(...);

	[optional] skb_reserve(skb, ...);

	... skb->tail ...

But even if the skb_reserve() happens, skb->data equals
skb->tail.  So it doesn't make any sense to use anything
other than skb->data in these cases.

Another case was the s2io.c driver directly mucking with
the skb->data and skb->tail pointers.  It really just wanted
to do an skb_reserve(), so that's what the code was changed
to do instead.

Another reason I'm making this change as it allows some SKB
cleanups I have planned simpler to merge.  In those cleanups,
skb->head, skb->tail, and skb->end pointers are removed, and
replaced with skb->head_room and skb->tail_room integers.

Signed-off-by: David S. Miller <davem@davemloft.net>
Acked-by: Jeff Garzik <jgarzik@pobox.com>
2005-06-28 15:25:31 -07:00
Domen Puncer
1e7f0bd8c8 drivers/net/: Use the DMA_{64,32}BIT_MASK constants
Use the DMA_{64,32}BIT_MASK constants from dma-mapping.h when calling
pci_set_dma_mask() or pci_set_consistent_dma_mask()

This patch includes dma-mapping.h explicitly because it caused errors
on some architectures otherwise.

See http://marc.theaimsgroup.com/?t=108001993000001&r=1&w=2 for details

Signed-off-by: Tobias Klauser <tklauser@nuerscht.ch>
Signed-off-by: Domen Puncer <domen@coderock.org>
2005-06-26 18:22:14 -04:00
Benjamin LaHaise
c16ef1ceed [PATCH] ns83820 update
Changes:
- improved DAC ifdefs from Andi Kleen
- removal of dead code from Adrian Bunk
- fix half duplex collision behaviour
2005-05-15 22:26:45 -04:00
Linus Torvalds
1da177e4c3 Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.

Let it rip!
2005-04-16 15:20:36 -07:00