netdev
[Top] [All Lists]

Re: BUG or not? GFP_KERNEL with interrupts disabled.

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


<Prev in Thread] Current Thread [Next in Thread>