* [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
@ 2011-06-28 18:58 Shyam Iyer
2011-06-28 23:32 ` Rasesh Mody
2011-07-07 15:13 ` Ivan Vecera
0 siblings, 2 replies; 7+ messages in thread
From: Shyam Iyer @ 2011-06-28 18:58 UTC (permalink / raw)
To: netdev; +Cc: rmody, ddutt, huangj, davem, Shyam Iyer
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.
Jun 27 08:15:24 home-t710 kernel: [11735.634550] Brocade 10G Ethernet driver
Jun 27 08:15:24 home-t710 kernel: [11735.634590] bnad_pci_probe : (0xffff880427f3d000, 0xffffffffa020f3e0) PCI Func : (2)
Jun 27 08:15:24 home-t710 kernel: [11735.637677] bna 0000:82:00.2: PCI INT A -> GSI 66 (level, low) -> IRQ 66
Jun 27 08:15:24 home-t710 kernel: [11735.638290] bar0 mapped to ffffc90014980000, len 262144
Jun 27 08:15:24 home-t710 kernel: [11735.638732] BUG: sleeping function called from invalid context at mm/slub.c:847
Jun 27 08:15:24 home-t710 kernel: [11735.638736] in_atomic(): 0, irqs_disabled(): 1, pid: 11243, name: insmod
Jun 27 08:15:24 home-t710 kernel: [11735.638740] Pid: 11243, comm: insmod Not tainted 3.0.0-rc4+ #6
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
Jun 27 08:15:24 home-t710 kernel: [11735.638766] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
Jun 27 08:15:24 home-t710 kernel: [11735.638773] [<ffffffff8111201c>] kmem_cache_alloc_trace+0x43/0xd8
Jun 27 08:15:24 home-t710 kernel: [11735.638782] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
Jun 27 08:15:24 home-t710 kernel: [11735.638787] [<ffffffff810ab791>] request_threaded_irq+0xa1/0x113
Jun 27 08:15:24 home-t710 kernel: [11735.638798] [<ffffffffa020f0c0>] bnad_pci_probe+0x612/0x8e5 [bna]
Jun 27 08:15:24 home-t710 kernel: [11735.638807] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
Jun 27 08:15:24 home-t710 kernel: [11735.638816] [<ffffffff81482ef4>] ? _raw_spin_unlock_irqrestore+0x17/0x19
Jun 27 08:15:24 home-t710 kernel: [11735.638822] [<ffffffff8124d17a>] local_pci_probe+0x44/0x75
Jun 27 08:15:24 home-t710 kernel: [11735.638826] [<ffffffff8124dc06>] pci_device_probe+0xd0/0xff
Jun 27 08:15:24 home-t710 kernel: [11735.638832] [<ffffffff812ef8ab>] driver_probe_device+0x131/0x213
Jun 27 08:15:24 home-t710 kernel: [11735.638836] [<ffffffff812ef9e7>] __driver_attach+0x5a/0x7e
Jun 27 08:15:24 home-t710 kernel: [11735.638840] [<ffffffff812ef98d>] ? driver_probe_device+0x213/0x213
Jun 27 08:15:24 home-t710 kernel: [11735.638844] [<ffffffff812ee933>] bus_for_each_dev+0x53/0x89
Jun 27 08:15:24 home-t710 kernel: [11735.638848] [<ffffffff812ef48a>] driver_attach+0x1e/0x20
Jun 27 08:15:24 home-t710 kernel: [11735.638852] [<ffffffff812ef0ae>] bus_add_driver+0xd1/0x224
Jun 27 08:15:24 home-t710 kernel: [11735.638858] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
Jun 27 08:15:24 home-t710 kernel: [11735.638862] [<ffffffff812efe57>] driver_register+0x98/0x105
Jun 27 08:15:24 home-t710 kernel: [11735.638866] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
Jun 27 08:15:24 home-t710 kernel: [11735.638871] [<ffffffff8124e4c9>] __pci_register_driver+0x56/0xc1
Jun 27 08:15:24 home-t710 kernel: [11735.638875] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
Jun 27 08:15:24 home-t710 kernel: [11735.638884] [<ffffffffa01b8040>] bnad_module_init+0x40/0x60 [bna]
Jun 27 08:15:24 home-t710 kernel: [11735.638892] [<ffffffff81002099>] do_one_initcall+0x7f/0x136
Jun 27 08:15:24 home-t710 kernel: [11735.638899] [<ffffffff8108608b>] sys_init_module+0x88/0x1d0
Jun 27 08:15:24 home-t710 kernel: [11735.638906] [<ffffffff81489682>] system_call_fastpath+0x16/0x1b
Jun 27 08:15:24 home-t710 kernel: [11735.639642] bnad_pci_probe : (0xffff880427f3e000, 0xffffffffa020f3e0) PCI Func : (3)
Jun 27 08:15:24 home-t710 kernel: [11735.639665] bna 0000:82:00.3: PCI INT A -> GSI 66 (level, low) -> IRQ 66
Jun 27 08:15:24 home-t710 kernel: [11735.639735] bar0 mapped to ffffc90014400000, len 262144
Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
---
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);
/*
--
1.7.5.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-06-28 18:58 [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called Shyam Iyer
@ 2011-06-28 23:32 ` Rasesh Mody
2011-07-02 0:50 ` David Miller
2011-07-07 15:13 ` Ivan Vecera
1 sibling, 1 reply; 7+ messages in thread
From: Rasesh Mody @ 2011-06-28 23:32 UTC (permalink / raw)
To: Shyam Iyer, netdev; +Cc: Debashis Dutt, Jing Huang, davem, Shyam Iyer
>From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com]
>Sent: Tuesday, June 28, 2011 11:58 AM
>
>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.
>
>@@ -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);
>
> /*
Patch looks fine.
Acked-by: Rasesh Mody <rmody@brocade.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-06-28 23:32 ` Rasesh Mody
@ 2011-07-02 0:50 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2011-07-02 0:50 UTC (permalink / raw)
To: rmody; +Cc: shyam.iyer.t, netdev, ddutt, huangj, shyam_iyer
From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 28 Jun 2011 16:32:58 -0700
>>From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com]
>>Sent: Tuesday, June 28, 2011 11:58 AM
>>
>>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.
...
> Patch looks fine.
>
> Acked-by: Rasesh Mody <rmody@brocade.com>
Applied, thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-06-28 18:58 [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called Shyam Iyer
2011-06-28 23:32 ` Rasesh Mody
@ 2011-07-07 15:13 ` Ivan Vecera
2011-07-07 21:53 ` Shyam_Iyer
2011-07-07 22:20 ` Rasesh Mody
1 sibling, 2 replies; 7+ messages in thread
From: Ivan Vecera @ 2011-07-07 15:13 UTC (permalink / raw)
To: Shyam Iyer; +Cc: netdev, rmody, ddutt, huangj, davem, Shyam Iyer
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.
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:
> 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.
>
> Jun 27 08:15:24 home-t710 kernel: [11735.634550] Brocade 10G Ethernet driver
> Jun 27 08:15:24 home-t710 kernel: [11735.634590] bnad_pci_probe : (0xffff880427f3d000, 0xffffffffa020f3e0) PCI Func : (2)
> Jun 27 08:15:24 home-t710 kernel: [11735.637677] bna 0000:82:00.2: PCI INT A -> GSI 66 (level, low) -> IRQ 66
> Jun 27 08:15:24 home-t710 kernel: [11735.638290] bar0 mapped to ffffc90014980000, len 262144
> Jun 27 08:15:24 home-t710 kernel: [11735.638732] BUG: sleeping function called from invalid context at mm/slub.c:847
> Jun 27 08:15:24 home-t710 kernel: [11735.638736] in_atomic(): 0, irqs_disabled(): 1, pid: 11243, name: insmod
> Jun 27 08:15:24 home-t710 kernel: [11735.638740] Pid: 11243, comm: insmod Not tainted 3.0.0-rc4+ #6
> 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
> Jun 27 08:15:24 home-t710 kernel: [11735.638766] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> Jun 27 08:15:24 home-t710 kernel: [11735.638773] [<ffffffff8111201c>] kmem_cache_alloc_trace+0x43/0xd8
> Jun 27 08:15:24 home-t710 kernel: [11735.638782] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> Jun 27 08:15:24 home-t710 kernel: [11735.638787] [<ffffffff810ab791>] request_threaded_irq+0xa1/0x113
> Jun 27 08:15:24 home-t710 kernel: [11735.638798] [<ffffffffa020f0c0>] bnad_pci_probe+0x612/0x8e5 [bna]
> Jun 27 08:15:24 home-t710 kernel: [11735.638807] [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> Jun 27 08:15:24 home-t710 kernel: [11735.638816] [<ffffffff81482ef4>] ? _raw_spin_unlock_irqrestore+0x17/0x19
> Jun 27 08:15:24 home-t710 kernel: [11735.638822] [<ffffffff8124d17a>] local_pci_probe+0x44/0x75
> Jun 27 08:15:24 home-t710 kernel: [11735.638826] [<ffffffff8124dc06>] pci_device_probe+0xd0/0xff
> Jun 27 08:15:24 home-t710 kernel: [11735.638832] [<ffffffff812ef8ab>] driver_probe_device+0x131/0x213
> Jun 27 08:15:24 home-t710 kernel: [11735.638836] [<ffffffff812ef9e7>] __driver_attach+0x5a/0x7e
> Jun 27 08:15:24 home-t710 kernel: [11735.638840] [<ffffffff812ef98d>] ? driver_probe_device+0x213/0x213
> Jun 27 08:15:24 home-t710 kernel: [11735.638844] [<ffffffff812ee933>] bus_for_each_dev+0x53/0x89
> Jun 27 08:15:24 home-t710 kernel: [11735.638848] [<ffffffff812ef48a>] driver_attach+0x1e/0x20
> Jun 27 08:15:24 home-t710 kernel: [11735.638852] [<ffffffff812ef0ae>] bus_add_driver+0xd1/0x224
> Jun 27 08:15:24 home-t710 kernel: [11735.638858] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> Jun 27 08:15:24 home-t710 kernel: [11735.638862] [<ffffffff812efe57>] driver_register+0x98/0x105
> Jun 27 08:15:24 home-t710 kernel: [11735.638866] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> Jun 27 08:15:24 home-t710 kernel: [11735.638871] [<ffffffff8124e4c9>] __pci_register_driver+0x56/0xc1
> Jun 27 08:15:24 home-t710 kernel: [11735.638875] [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> Jun 27 08:15:24 home-t710 kernel: [11735.638884] [<ffffffffa01b8040>] bnad_module_init+0x40/0x60 [bna]
> Jun 27 08:15:24 home-t710 kernel: [11735.638892] [<ffffffff81002099>] do_one_initcall+0x7f/0x136
> Jun 27 08:15:24 home-t710 kernel: [11735.638899] [<ffffffff8108608b>] sys_init_module+0x88/0x1d0
> Jun 27 08:15:24 home-t710 kernel: [11735.638906] [<ffffffff81489682>] system_call_fastpath+0x16/0x1b
> Jun 27 08:15:24 home-t710 kernel: [11735.639642] bnad_pci_probe : (0xffff880427f3e000, 0xffffffffa020f3e0) PCI Func : (3)
> Jun 27 08:15:24 home-t710 kernel: [11735.639665] bna 0000:82:00.3: PCI INT A -> GSI 66 (level, low) -> IRQ 66
> Jun 27 08:15:24 home-t710 kernel: [11735.639735] bar0 mapped to ffffc90014400000, len 262144
>
> Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
> ---
> 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);
>
> /*
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-07-07 15:13 ` Ivan Vecera
@ 2011-07-07 21:53 ` Shyam_Iyer
2011-07-12 12:42 ` Ivan Vecera
2011-07-07 22:20 ` Rasesh Mody
1 sibling, 1 reply; 7+ messages in thread
From: Shyam_Iyer @ 2011-07-07 21:53 UTC (permalink / raw)
To: ivecera, shyam.iyer.t; +Cc: netdev, rmody, ddutt, huangj, davem
> -----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:
> > 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.
> >
> > Jun 27 08:15:24 home-t710 kernel: [11735.634550] Brocade 10G Ethernet
> driver
> > Jun 27 08:15:24 home-t710 kernel: [11735.634590] bnad_pci_probe :
> (0xffff880427f3d000, 0xffffffffa020f3e0) PCI Func : (2)
> > Jun 27 08:15:24 home-t710 kernel: [11735.637677] bna 0000:82:00.2:
> PCI INT A -> GSI 66 (level, low) -> IRQ 66
> > Jun 27 08:15:24 home-t710 kernel: [11735.638290] bar0 mapped to
> ffffc90014980000, len 262144
> > Jun 27 08:15:24 home-t710 kernel: [11735.638732] BUG: sleeping
> function called from invalid context at mm/slub.c:847
> > Jun 27 08:15:24 home-t710 kernel: [11735.638736] in_atomic(): 0,
> irqs_disabled(): 1, pid: 11243, name: insmod
> > Jun 27 08:15:24 home-t710 kernel: [11735.638740] Pid: 11243, comm:
> insmod Not tainted 3.0.0-rc4+ #6
> > 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
> > Jun 27 08:15:24 home-t710 kernel: [11735.638766]
> [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> > Jun 27 08:15:24 home-t710 kernel: [11735.638773]
> [<ffffffff8111201c>] kmem_cache_alloc_trace+0x43/0xd8
> > Jun 27 08:15:24 home-t710 kernel: [11735.638782]
> [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> > Jun 27 08:15:24 home-t710 kernel: [11735.638787]
> [<ffffffff810ab791>] request_threaded_irq+0xa1/0x113
> > Jun 27 08:15:24 home-t710 kernel: [11735.638798]
> [<ffffffffa020f0c0>] bnad_pci_probe+0x612/0x8e5 [bna]
> > Jun 27 08:15:24 home-t710 kernel: [11735.638807]
> [<ffffffffa01fe469>] ? netif_wake_queue+0x3d/0x3d [bna]
> > Jun 27 08:15:24 home-t710 kernel: [11735.638816]
> [<ffffffff81482ef4>] ? _raw_spin_unlock_irqrestore+0x17/0x19
> > Jun 27 08:15:24 home-t710 kernel: [11735.638822]
> [<ffffffff8124d17a>] local_pci_probe+0x44/0x75
> > Jun 27 08:15:24 home-t710 kernel: [11735.638826]
> [<ffffffff8124dc06>] pci_device_probe+0xd0/0xff
> > Jun 27 08:15:24 home-t710 kernel: [11735.638832]
> [<ffffffff812ef8ab>] driver_probe_device+0x131/0x213
> > Jun 27 08:15:24 home-t710 kernel: [11735.638836]
> [<ffffffff812ef9e7>] __driver_attach+0x5a/0x7e
> > Jun 27 08:15:24 home-t710 kernel: [11735.638840]
> [<ffffffff812ef98d>] ? driver_probe_device+0x213/0x213
> > Jun 27 08:15:24 home-t710 kernel: [11735.638844]
> [<ffffffff812ee933>] bus_for_each_dev+0x53/0x89
> > Jun 27 08:15:24 home-t710 kernel: [11735.638848]
> [<ffffffff812ef48a>] driver_attach+0x1e/0x20
> > Jun 27 08:15:24 home-t710 kernel: [11735.638852]
> [<ffffffff812ef0ae>] bus_add_driver+0xd1/0x224
> > Jun 27 08:15:24 home-t710 kernel: [11735.638858]
> [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> > Jun 27 08:15:24 home-t710 kernel: [11735.638862]
> [<ffffffff812efe57>] driver_register+0x98/0x105
> > Jun 27 08:15:24 home-t710 kernel: [11735.638866]
> [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> > Jun 27 08:15:24 home-t710 kernel: [11735.638871]
> [<ffffffff8124e4c9>] __pci_register_driver+0x56/0xc1
> > Jun 27 08:15:24 home-t710 kernel: [11735.638875]
> [<ffffffffa01b8000>] ? 0xffffffffa01b7fff
> > Jun 27 08:15:24 home-t710 kernel: [11735.638884]
> [<ffffffffa01b8040>] bnad_module_init+0x40/0x60 [bna]
> > Jun 27 08:15:24 home-t710 kernel: [11735.638892]
> [<ffffffff81002099>] do_one_initcall+0x7f/0x136
> > Jun 27 08:15:24 home-t710 kernel: [11735.638899]
> [<ffffffff8108608b>] sys_init_module+0x88/0x1d0
> > Jun 27 08:15:24 home-t710 kernel: [11735.638906]
> [<ffffffff81489682>] system_call_fastpath+0x16/0x1b
> > Jun 27 08:15:24 home-t710 kernel: [11735.639642] bnad_pci_probe :
> (0xffff880427f3e000, 0xffffffffa020f3e0) PCI Func : (3)
> > Jun 27 08:15:24 home-t710 kernel: [11735.639665] bna 0000:82:00.3:
> PCI INT A -> GSI 66 (level, low) -> IRQ 66
> > Jun 27 08:15:24 home-t710 kernel: [11735.639735] bar0 mapped to
> ffffc90014400000, len 262144
> >
> > Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
> > ---
> > 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);
> >
> > /*
>
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-07-07 15:13 ` Ivan Vecera
2011-07-07 21:53 ` Shyam_Iyer
@ 2011-07-07 22:20 ` Rasesh Mody
1 sibling, 0 replies; 7+ messages in thread
From: Rasesh Mody @ 2011-07-07 22:20 UTC (permalink / raw)
To: Ivan Vecera, Shyam Iyer
Cc: netdev, Debashis Dutt, Jing Huang, davem, Shyam Iyer
Ivan,
I agree with you regarding use of flags varible used for two different purposes. It is creating some confusion. We can have request_irq function use irq_flags varible. This will ensure flags varible is exclusively used for enabling/disabling IRQs.
Thanks,
Rasesh
>-----Original Message-----
>From: Ivan Vecera [mailto:ivecera@redhat.com]
>Sent: Thursday, July 07, 2011 8:14 AM
>To: Shyam Iyer
>Cc: netdev@vger.kernel.org; Rasesh Mody; Debashis Dutt; Jing Huang;
>davem@davemloft.net; Shyam Iyer
>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.
>
>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:
>> 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.
>>
>> Jun 27 08:15:24 home-t710 kernel: [11735.634550] Brocade 10G Ethernet
>driver
>> Jun 27 08:15:24 home-t710 kernel: [11735.634590] bnad_pci_probe :
>(0xffff880427f3d000, 0xffffffffa020f3e0) PCI Func : (2)
>> Jun 27 08:15:24 home-t710 kernel: [11735.637677] bna 0000:82:00.2: PCI
>INT A -> GSI 66 (level, low) -> IRQ 66
>> Jun 27 08:15:24 home-t710 kernel: [11735.638290] bar0 mapped to
>ffffc90014980000, len 262144
>> Jun 27 08:15:24 home-t710 kernel: [11735.638732] BUG: sleeping
>function called from invalid context at mm/slub.c:847
>> Jun 27 08:15:24 home-t710 kernel: [11735.638736] in_atomic(): 0,
>irqs_disabled(): 1, pid: 11243, name: insmod
>> Jun 27 08:15:24 home-t710 kernel: [11735.638740] Pid: 11243, comm:
>insmod Not tainted 3.0.0-rc4+ #6
>> 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
>> Jun 27 08:15:24 home-t710 kernel: [11735.638766] [<ffffffffa01fe469>]
>? netif_wake_queue+0x3d/0x3d [bna]
>> Jun 27 08:15:24 home-t710 kernel: [11735.638773] [<ffffffff8111201c>]
>kmem_cache_alloc_trace+0x43/0xd8
>> Jun 27 08:15:24 home-t710 kernel: [11735.638782] [<ffffffffa01fe469>]
>? netif_wake_queue+0x3d/0x3d [bna]
>> Jun 27 08:15:24 home-t710 kernel: [11735.638787] [<ffffffff810ab791>]
>request_threaded_irq+0xa1/0x113
>> Jun 27 08:15:24 home-t710 kernel: [11735.638798] [<ffffffffa020f0c0>]
>bnad_pci_probe+0x612/0x8e5 [bna]
>> Jun 27 08:15:24 home-t710 kernel: [11735.638807] [<ffffffffa01fe469>]
>? netif_wake_queue+0x3d/0x3d [bna]
>> Jun 27 08:15:24 home-t710 kernel: [11735.638816] [<ffffffff81482ef4>]
>? _raw_spin_unlock_irqrestore+0x17/0x19
>> Jun 27 08:15:24 home-t710 kernel: [11735.638822] [<ffffffff8124d17a>]
>local_pci_probe+0x44/0x75
>> Jun 27 08:15:24 home-t710 kernel: [11735.638826] [<ffffffff8124dc06>]
>pci_device_probe+0xd0/0xff
>> Jun 27 08:15:24 home-t710 kernel: [11735.638832] [<ffffffff812ef8ab>]
>driver_probe_device+0x131/0x213
>> Jun 27 08:15:24 home-t710 kernel: [11735.638836] [<ffffffff812ef9e7>]
>__driver_attach+0x5a/0x7e
>> Jun 27 08:15:24 home-t710 kernel: [11735.638840] [<ffffffff812ef98d>]
>? driver_probe_device+0x213/0x213
>> Jun 27 08:15:24 home-t710 kernel: [11735.638844] [<ffffffff812ee933>]
>bus_for_each_dev+0x53/0x89
>> Jun 27 08:15:24 home-t710 kernel: [11735.638848] [<ffffffff812ef48a>]
>driver_attach+0x1e/0x20
>> Jun 27 08:15:24 home-t710 kernel: [11735.638852] [<ffffffff812ef0ae>]
>bus_add_driver+0xd1/0x224
>> Jun 27 08:15:24 home-t710 kernel: [11735.638858] [<ffffffffa01b8000>]
>? 0xffffffffa01b7fff
>> Jun 27 08:15:24 home-t710 kernel: [11735.638862] [<ffffffff812efe57>]
>driver_register+0x98/0x105
>> Jun 27 08:15:24 home-t710 kernel: [11735.638866] [<ffffffffa01b8000>]
>? 0xffffffffa01b7fff
>> Jun 27 08:15:24 home-t710 kernel: [11735.638871] [<ffffffff8124e4c9>]
>__pci_register_driver+0x56/0xc1
>> Jun 27 08:15:24 home-t710 kernel: [11735.638875] [<ffffffffa01b8000>]
>? 0xffffffffa01b7fff
>> Jun 27 08:15:24 home-t710 kernel: [11735.638884] [<ffffffffa01b8040>]
>bnad_module_init+0x40/0x60 [bna]
>> Jun 27 08:15:24 home-t710 kernel: [11735.638892] [<ffffffff81002099>]
>do_one_initcall+0x7f/0x136
>> Jun 27 08:15:24 home-t710 kernel: [11735.638899] [<ffffffff8108608b>]
>sys_init_module+0x88/0x1d0
>> Jun 27 08:15:24 home-t710 kernel: [11735.638906] [<ffffffff81489682>]
>system_call_fastpath+0x16/0x1b
>> Jun 27 08:15:24 home-t710 kernel: [11735.639642] bnad_pci_probe :
>(0xffff880427f3e000, 0xffffffffa020f3e0) PCI Func : (3)
>> Jun 27 08:15:24 home-t710 kernel: [11735.639665] bna 0000:82:00.3: PCI
>INT A -> GSI 66 (level, low) -> IRQ 66
>> Jun 27 08:15:24 home-t710 kernel: [11735.639735] bar0 mapped to
>ffffc90014400000, len 262144
>>
>> Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
>> ---
>> 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);
>>
>> /*
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
2011-07-07 21:53 ` Shyam_Iyer
@ 2011-07-12 12:42 ` Ivan Vecera
0 siblings, 0 replies; 7+ messages in thread
From: Ivan Vecera @ 2011-07-12 12:42 UTC (permalink / raw)
To: Shyam_Iyer; +Cc: shyam.iyer.t, netdev, rmody, ddutt, huangj, davem
On Fri, 2011-07-08 at 03:23 +0530, Shyam_Iyer@Dell.com wrote:
>
> > -----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)
>
Yes, this is much cleaner...
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-07-12 12:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-28 18:58 [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called Shyam Iyer
2011-06-28 23:32 ` Rasesh Mody
2011-07-02 0:50 ` David Miller
2011-07-07 15:13 ` Ivan Vecera
2011-07-07 21:53 ` Shyam_Iyer
2011-07-12 12:42 ` Ivan Vecera
2011-07-07 22:20 ` Rasesh Mody
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.