-----Original Message-----
From: Ivan Vecera [mailto:ivecera@redhat.com]
Sent: Thursday, July 07, 2011 11:14 AM
To: Shyam Iyer
Cc: netdev@vger.kernel.org; rmody@brocade.com; ddutt@brocade.com;
huangj@brocade.com; davem@davemloft.net; Iyer, Shyam
Subject: Re: [PATCH] [v3][net][bna] Fix call trace when interrupts are
disabled while sleeping function kzalloc is called

Small note, the root of the problem was that non-atomic allocation was
requested with IRQs disabled. Your patch description does not contain
why were the IRQs disabled.
The function bnad_mbox_irq_alloc incorrectly uses 'flags' var for two
different things, 1) to save current CPU flags and 2) for request_irq
call.
First the spin_lock_irqsave disables the IRQs and saves _all_ CPU flags
(including one that enables/disables interrupts) to 'flags'. Then the
'flags' is overwritten by 0 or 0x80 (IRQF_SHARED). Finally the
spin_unlock_irqrestore should restore saved flags, but these flags are
now either 0x00 or 0x80. The interrupt bit value in flags register on
x86 arch is 0x100.
This means that the interrupt bit is zero (IRQs disabled) after
spin_unlock_irqrestore so the request_irq function is called with
disabled interrupts.

That seems to make a lot more sense.. and that way I don't have to initialize the flags variable outside of the spin_lock_irqsave/restore bracket to preserve the irqs not being disabled when allocating in request_irq

Would the below patch make sense instead...? (Note that since davem has accepted the earlier one this is on top of the already committed patch)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 44e219c..ea13605 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c

@@ -1111,7 +1111,7 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
                    struct bna_intr_info *intr_info)
 {
        int             err = 0;
-       unsigned long   irq_flags = 0, flags;
+       unsigned long   irq_flags, flags;
        u32     irq;
        irq_handler_t   irq_handler;
 

@@ -1125,6 +1125,7 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
        if (bnad->cfg_flags & BNAD_CF_MSIX) {
                irq_handler = (irq_handler_t)bnad_msix_mbox_handler;
                irq = bnad->msix_table[bnad->msix_num - 1].vector;
+               irq_flags = 0;
                intr_info->intr_type = BNA_INTR_T_MSIX;
                intr_info->idl[0].vector = bnad->msix_num - 1;
        } else {

@@ -1135,7 +1136,6 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
                /* intr_info->idl.vector = 0 ? */
        }
        spin_unlock_irqrestore(&bnad->bna_lock, flags);
-       flags = irq_flags;
        sprintf(bnad->mbox_irq_name, "%s", BNAD_NAME);
 
        /*

@@ -1146,7 +1146,7 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
 
        BNAD_UPDATE_CTR(bnad, mbox_intr_disabled);
 
-       err = request_irq(irq, irq_handler, flags,
+       err = request_irq(irq, irq_handler, irq_flags,
                          bnad->mbox_irq_name, bnad);
 
        if (err) {

(END)

Rasesh, this is not a good idea to use one variable for two different
purposes in parallel.

Regards,
Ivan

On Tue, 2011-06-28 at 14:58 -0400, Shyam Iyer wrote:

quoted

request_threaded irq will call kzalloc that can sleep. Initializing

the flags variable outside of spin_lock_irqsave/restore in
bnad_mbox_irq_alloc will avoid call traces like below.

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.634550] Brocade 10G Ethernet

driver

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.634590] bnad_pci_probe :

(0xffff880427f3d000, 0xffffffffa020f3e0) PCI Func : (2)

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.637677] bna 0000:82:00.2:

PCI INT A -> GSI 66 (level, low) -> IRQ 66

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638290] bar0 mapped to

ffffc90014980000, len 262144

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638732] BUG: sleeping

function called from invalid context at mm/slub.c:847

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638736] in_atomic(): 0,

irqs_disabled(): 1, pid: 11243, name: insmod

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638740] Pid: 11243, comm:

insmod Not tainted 3.0.0-rc4+ #6

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638743] Call Trace:
Jun 27 08:15:24 home-t710 kernel: [11735.638755]

[<ffffffff81046427>] __might_sleep+0xeb/0xf0

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638766]

[<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638773]

[<ffffffff8111201c>] kmem_cache_alloc_trace+0x43/0xd8

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638782]

[<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638787]

[<ffffffff810ab791>] request_threaded_irq+0xa1/0x113

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638798]

[<ffffffffa020f0c0>] bnad_pci_probe+0x612/0x8e5 [bna]

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638807]

[<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638816]

[<ffffffff81482ef4>] ? _raw_spin_unlock_irqrestore+0x17/0x19

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638822]

[<ffffffff8124d17a>] local_pci_probe+0x44/0x75

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638826]

[<ffffffff8124dc06>] pci_device_probe+0xd0/0xff

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638832]

[<ffffffff812ef8ab>] driver_probe_device+0x131/0x213

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638836]

[<ffffffff812ef9e7>] __driver_attach+0x5a/0x7e

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638840]

[<ffffffff812ef98d>] ? driver_probe_device+0x213/0x213

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638844]

[<ffffffff812ee933>] bus_for_each_dev+0x53/0x89

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638848]

[<ffffffff812ef48a>] driver_attach+0x1e/0x20

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638852]

[<ffffffff812ef0ae>] bus_add_driver+0xd1/0x224

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638858]

[<ffffffffa01b8000>] ? 0xffffffffa01b7fff

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638862]

[<ffffffff812efe57>] driver_register+0x98/0x105

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638866]

[<ffffffffa01b8000>] ? 0xffffffffa01b7fff

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638871]

[<ffffffff8124e4c9>] __pci_register_driver+0x56/0xc1

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638875]

[<ffffffffa01b8000>] ? 0xffffffffa01b7fff

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638884]

[<ffffffffa01b8040>] bnad_module_init+0x40/0x60 [bna]

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638892]

[<ffffffff81002099>] do_one_initcall+0x7f/0x136

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638899]

[<ffffffff8108608b>] sys_init_module+0x88/0x1d0

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.638906]

[<ffffffff81489682>] system_call_fastpath+0x16/0x1b

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.639642] bnad_pci_probe :

(0xffff880427f3e000, 0xffffffffa020f3e0) PCI Func : (3)

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.639665] bna 0000:82:00.3:

PCI INT A -> GSI 66 (level, low) -> IRQ 66

quoted

Jun 27 08:15:24 home-t710 kernel: [11735.639735] bar0 mapped to

ffffc90014400000, len 262144

quoted

Signed-off-by: Shyam Iyer <redacted>
---
 drivers/net/bna/bnad.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c
index 7d25a97..44e219c 100644
--- a/drivers/net/bna/bnad.c
+++ b/drivers/net/bna/bnad.c

@@ -1111,7 +1111,7 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
 		    struct bna_intr_info *intr_info)
 {
 	int 		err = 0;
-	unsigned long 	flags;
+	unsigned long 	irq_flags = 0, flags;
 	u32	irq;
 	irq_handler_t 	irq_handler;

@@ -1125,18 +1125,17 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
 	if (bnad->cfg_flags & BNAD_CF_MSIX) {
 		irq_handler = (irq_handler_t)bnad_msix_mbox_handler;
 		irq = bnad->msix_table[bnad->msix_num - 1].vector;
-		flags = 0;
 		intr_info->intr_type = BNA_INTR_T_MSIX;
 		intr_info->idl[0].vector = bnad->msix_num - 1;
 	} else {
 		irq_handler = (irq_handler_t)bnad_isr;
 		irq = bnad->pcidev->irq;
-		flags = IRQF_SHARED;
+		irq_flags = IRQF_SHARED;
 		intr_info->intr_type = BNA_INTR_T_INTX;
 		/* intr_info->idl.vector = 0 ? */
 	}
 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
-
+	flags = irq_flags;
 	sprintf(bnad->mbox_irq_name, "%s", BNAD_NAME);

 	/*