| To: | Dan Eble <dane@xxxxxxxxxx> |
|---|---|
| Subject: | Re: BUG or not? GFP_KERNEL with interrupts disabled. |
| From: | Linus Torvalds <torvalds@xxxxxxxxxxxxx> |
| Date: | Thu, 27 Mar 2003 11:08:26 -0800 (PST) |
| Cc: | "David S. Miller" <davem@xxxxxxxxxx>, <shmulik.hen@xxxxxxxxx>, <bonding-devel@xxxxxxxxxxxxxxxxxxxxx>, <bonding-announce@xxxxxxxxxxxxxxxxxxxxx>, <netdev@xxxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <linux-net@xxxxxxxxxxxxxxx>, <mingo@xxxxxxxxxx>, <kuznet@xxxxxxxxxxxxx> |
| In-reply-to: | <Pine.LNX.4.33.0303271315010.30532-100000@xxxxxxxxxxxxxxxxxxxxx> |
| Sender: | netdev-bounce@xxxxxxxxxxx |
On Thu, 27 Mar 2003, Dan Eble wrote:
>
> This makes checks like the following (in alloc_skb) asymmetric:
>
> if (in_interrupt() && (gfp_mask & __GFP_WAIT)) {
> static int count = 0;
> if (++count < 5) {
> printk(KERN_ERR "alloc_skb called nonatomically "
> "from interrupt %p\n", NET_CALLER(size));
> BUG();
>
> In a driver I'm writing, this bug was hidden until I switched from using
> write_lock_irqsave() to write_lock_bh(). Shouldn't this bug also be
> announced if interrupts are disabled?
Yeah. It should also probably use "in_atomic()" instead of
"in_interrupt()", since that also finds people who have marked themselves
non-preemptible.
So what the test SHOULD look like is this:
if (gfp_mask & __GFP_WAIT) {
if (in_atomic() || irqs_disabled()) {
static int count = 0;
...
}
}
which should catch all the cases we really care about.
Linus
|
| Previous by Date: | Re: BUG or not? GFP_KERNEL with interrupts disabled., Dan Eble |
|---|---|
| Next by Date: | Re: BUG or not? GFP_KERNEL with interrupts disabled., David S. Miller |
| Previous by Thread: | Re: BUG or not? GFP_KERNEL with interrupts disabled., Dan Eble |
| Next by Thread: | Re: BUG or not? GFP_KERNEL with interrupts disabled., David S. Miller |
| Indexes: | [Date] [Thread] [Top] [All Lists] |