Re: [PATCH 2.6.16] Shared interrupts sometimes lost

[Neil Brown <neilb@suse.de>]

On Tuesday April 11, pavel@suse.cz wrote:
> > 
> >  Now if the IRQ is handled as an edge-triggered line (which I believe
> >  they are in Linux), then losing this race will mean that we don't see
> >  any more interrupts on this line.
> 
> I believe that
> 
> a) any shared interrupts should be level-triggered. It is not okay to
> share edge-triggered interrupt

I see that now, thanks.

> 
> b) your patch does not fix that issue. It only makes race window
> smaller.
> 
> >  	if (!(action->flags & SA_INTERRUPT))
> >  		local_irq_enable();
> >  
> >  	do {
> >  		ret = action->handler(irq, action->dev_id, regs);
> > -		if (ret == IRQ_HANDLED)
> > +		if (ret == IRQ_HANDLED) {
> >  			status |= action->flags;
> > +			repeat = 1;
> > +		}
> >  		retval |= ret;
> >  		action = action->next;
> > +		if (!action &&
> > +		    repeat &&
> > +		    safeirq &&
> > +		    (actionlist->flags & SA_SHIRQ)) {
> > +			/* at least one handler on the list did something,
> > +			 * and the interrupt is sharable, so give
> > +			 * every handler another chance, incase a new event
> > +			 * came in and is holding the irq line asserted.
> > +			 */
> > +			action = actionlist;
> > +			repeat = 0;
> > +		}
> >  	} while (action);
> 
> I think it is still racy. What if another interrupt comes here?

Well... at this point we are certain that the irq line was low at some
point since the loop was first entered, otherwise we would not have
been able to pass through all actions without getting a single
IRQ_HANDLED.
So if an interrupt happens now, it will generate an edge.
However I concede that I don't know how the PIC works exactly.  The
interrupt is still disabled in the PIC at this point.  If this means
that the edge will be ignored, then you are right.  If it just means
that the edge will be "delayed" and we will see the interrupt when
they are re-enabled at the PIC, then we should be safe.


> 
> >  	if (status & SA_SAMPLE_RANDOM)
> 
> 								Pavel
> -- 
> Thanks for all the (sleeping) penguins.

Do you have a gallery somewhere ???

Thanks for your input.

NeilBrown
-
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/