Skip to content

Commit

Permalink
Separate handling of irq type flags variable from the irq_flags reque…
Browse files Browse the repository at this point in the history
…st_irq variable

Commit 5f77898 does not completely
fix the problem of handling allocations with irqs disabled..  The
below patch on top of it fixes the problem completely.

Based on review by "Ivan Vecera" <ivecera@redhat.com>..
"
Small note, the root of the problem was that non-atomic allocation was requested with IRQs disabled. Your patch description does not contain wwhy 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.
"

Signed-off-by: Shyam Iyer <shyam_iyer@dell.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Shyam Iyer authored and David S. Miller committed Jul 15, 2011
1 parent ecae42d commit 8279171
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions drivers/net/bna/bnad.c
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,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;

Expand All @@ -1123,6 +1123,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 {
Expand All @@ -1133,7 +1134,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);

/*
Expand All @@ -1144,7 +1144,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) {
Expand Down

0 comments on commit 8279171

Please sign in to comment.