Messages in this thread | | | Date | Thu, 26 Aug 2004 16:27:00 +0200 | From | Takashi Iwai <> | Subject | Re: [PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM |
| |
At Thu, 26 Aug 2004 15:18:32 +0100, Russell King wrote: > > On Thu, Aug 26, 2004 at 04:10:54PM +0200, Takashi Iwai wrote: > > At Thu, 26 Aug 2004 15:04:04 +0100, > > Russell King wrote: > > > > > > On Thu, Aug 26, 2004 at 02:50:52PM +0200, Takashi Iwai wrote: > > > > At Wed, 25 Aug 2004 13:41:12 -0700, > > > > Andrew Morton wrote: > > > > > > > > > > Takashi Iwai <tiwai@suse.de> wrote: > > > > > > > > > > > > Anyway, suppressing the unnecessary call of add_interrupt_randomness() > > > > > > should be still valid. The reduced patch is below. > > > > (snip) > > > > > > > > > > Shouldn't that be `if (ret == IRQ_HANDLED)'? > > > > > > > > Yes, it's more strict. > > > > > > I don't think so. Look at what's going on. If "ret" is IRQ_HANDLED > > > all well and fine. However, look at how "retval" is being used: > > > > > > static void __report_bad_irq(int irq, irq_desc_t *desc, irqreturn_t action_ret) > > > { > > > ... > > > if (action_ret != IRQ_HANDLED && action_ret != IRQ_NONE) { > > > printk(KERN_ERR "irq event %d: bogus return value %x\n", > > > irq, action_ret); > > > } else { > > > printk(KERN_ERR "irq %d: nobody cared!\n", irq); > > > } > > > > > > So, we're looking to see not only if a handler returned IRQ_HANDLED, > > > but also if a handler returned _some other value_ other than IRQ_HANDLED > > > or IRQ_NONE. > > > > But obviously any other value is invalid as shown above, so we > > shouldn't take it seriously as the correct return value for > > triggering add_random_interrupt(). > > Point is: you're killing the method for detecting when IRQ handlers > return bogus return codes since you only look at them when they > respond with IRQ_HANDLED. > > So, you're disabling the "bogus return value" check. Completely.
No, the return value from handle_IRQ_event won't be changed at all by my patch.
asmlinkage int handle_IRQ_event(unsigned int irq, ... { ... do { ret = action->handler(irq, action->dev_id, regs); ==> if (ret == IRQ_HANDLED) ==> status |= action->flags; ==> retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) add_interrupt_randomness(irq); local_irq_disable(); return retval; }
The patch adds the additional check for another variable 'status' which is used only for checking SA_SAMPLE_RANDOM.
Takashi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |