Re: [PATCH] free_irq(): Actually handle DEBUG_SHIRQ
On Wed, 2007-09-12 at 12:17 +0100, Maciej W. Rozycki wrote: > Code used for DEBUG_SHIRQ in free_irq() is unreachable -- the for() > loop within never terminates otherwise than by return. This is an > obvious fix. Yeah... except that it enables new debugging code which (due to my stupidity) wasn't enabled before. A fix for this is already in the -mm tree, to be merged (along with a corresponding lockdep-related fix) after 2.6.23 release. -- dwmw2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] free_irq(): Actually handle DEBUG_SHIRQ
Code used for DEBUG_SHIRQ in free_irq() is unreachable -- the for() loop within never terminates otherwise than by return. This is an obvious fix. Signed-off-by: Maciej W. Rozycki <[EMAIL PROTECTED]> --- Please apply. While at it, I have a question about the complementary code within request_irq(): when the option in question is enabled, a spurious IRQ is posted before internal setup is done by setup_irq(). This includes the ->depth counter. Now if the interrupt handler of some driver calls disable_irq_nosync(), then code in setup_irq() will mess up the state of ->depth afterwards if this is the first handler being installed (i.e. within the "if (!shared)" block). This is reported later on when enable_irq() called by the driver, by means of a message like this: Unbalanced enable for IRQ 34 WARNING: at kernel/irq/manage.c:158 enable_irq() Now given DEBUG_SHIRQ is a debug facility, not to be normally used for production, is the phenomenon as described considered a design limitation we can live with, or is it a bug that qualifies for a fix? I can reproduce this problem easily with phylib. Maciej patch-mips-2.6.23-rc5-20070904-debug-shirq-0 diff -up --recursive --new-file linux-mips-2.6.23-rc5-20070904.macro/kernel/irq/manage.c linux-mips-2.6.23-rc5-20070904/kernel/irq/manage.c --- linux-mips-2.6.23-rc5-20070904.macro/kernel/irq/manage.c2007-09-04 04:56:21.0 + +++ linux-mips-2.6.23-rc5-20070904/kernel/irq/manage.c 2007-09-11 23:58:59.0 + @@ -448,7 +448,7 @@ void free_irq(unsigned int irq, void *de if (action->flags & IRQF_SHARED) handler = action->handler; kfree(action); - return; + break; } printk(KERN_ERR "Trying to free already-free IRQ %d\n", irq); spin_unlock_irqrestore(>lock, flags); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] free_irq(): Actually handle DEBUG_SHIRQ
Code used for DEBUG_SHIRQ in free_irq() is unreachable -- the for() loop within never terminates otherwise than by return. This is an obvious fix. Signed-off-by: Maciej W. Rozycki [EMAIL PROTECTED] --- Please apply. While at it, I have a question about the complementary code within request_irq(): when the option in question is enabled, a spurious IRQ is posted before internal setup is done by setup_irq(). This includes the -depth counter. Now if the interrupt handler of some driver calls disable_irq_nosync(), then code in setup_irq() will mess up the state of -depth afterwards if this is the first handler being installed (i.e. within the if (!shared) block). This is reported later on when enable_irq() called by the driver, by means of a message like this: Unbalanced enable for IRQ 34 WARNING: at kernel/irq/manage.c:158 enable_irq() Now given DEBUG_SHIRQ is a debug facility, not to be normally used for production, is the phenomenon as described considered a design limitation we can live with, or is it a bug that qualifies for a fix? I can reproduce this problem easily with phylib. Maciej patch-mips-2.6.23-rc5-20070904-debug-shirq-0 diff -up --recursive --new-file linux-mips-2.6.23-rc5-20070904.macro/kernel/irq/manage.c linux-mips-2.6.23-rc5-20070904/kernel/irq/manage.c --- linux-mips-2.6.23-rc5-20070904.macro/kernel/irq/manage.c2007-09-04 04:56:21.0 + +++ linux-mips-2.6.23-rc5-20070904/kernel/irq/manage.c 2007-09-11 23:58:59.0 + @@ -448,7 +448,7 @@ void free_irq(unsigned int irq, void *de if (action-flags IRQF_SHARED) handler = action-handler; kfree(action); - return; + break; } printk(KERN_ERR Trying to free already-free IRQ %d\n, irq); spin_unlock_irqrestore(desc-lock, flags); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] free_irq(): Actually handle DEBUG_SHIRQ
On Wed, 2007-09-12 at 12:17 +0100, Maciej W. Rozycki wrote: Code used for DEBUG_SHIRQ in free_irq() is unreachable -- the for() loop within never terminates otherwise than by return. This is an obvious fix. Yeah... except that it enables new debugging code which (due to my stupidity) wasn't enabled before. A fix for this is already in the -mm tree, to be merged (along with a corresponding lockdep-related fix) after 2.6.23 release. -- dwmw2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
