lkml.org 
[lkml]   [2004]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.034 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site