Re: [RFC][PATCH] IRQ: Fix oneshot irq race between irq_finalize_oneshot and handle_level_irq

[Yong Zhang <yong.zhang@windriver.com>]

On Wed, Mar 10, 2010 at 08:56:22AM +0100, Thomas Gleixner wrote:
> > How about the following patch(maybe a little ugly). I think it will
> > resolve your concerns.
> 
> No it does not, but you are right that it's ugly. And it is patently
> wrong as well.
>  
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index d70394f..23b79c6 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -461,9 +461,24 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
> >  	raw_spin_lock(&desc->lock);
> >  	mask_ack_irq(desc, irq);

I must say this patch isn't based on your previous one in which
mask_ack_irq() is modified to flag IRQ_MASKED.
I let IRQ_MASKED serialise interrupt-handler and irq-thread in oneshot
mode. 

> >  
> > -	if (unlikely(desc->status & IRQ_INPROGRESS))
> > -		goto out_unlock;
> > +	/*
> > +	 * if we are in oneshot mode and the irq thread is running on
> > +	 * another cpu, just return because the irq thread will unmask
> > +	 * the irq
> > +	 */
> > +	if (unlikely(desc->status & IRQ_ONESHOT)) {
> > +		if (unlikely(desc->status & (IRQ_INPROGRESS | IRQ_MASKED)
> > +					  == IRQ_INPROGRESS | IRQ_MASKED))
> > +			goto out_unlock;
> > +	}
> > +	else {
> > +		if (unlikely(desc->status & IRQ_INPROGRESS))
> > +			goto out_unlock;
> > +	}
> 
>   In case of IRQ_SHOT and IRQ_INPROGRESS and the other CPU having
>   unmasked the interrupt already you are reentering the handler which
>   is a nono.

I have thought of that kind of reentering you point above,
if IRQ_MASKED is cleared by irq_finalize_oneshot(), the thread_fn()
is done as well as the hardware opration. If another irq comes in,
the reentering does happen, but I think it's harmless because at
this time we just set IRQTF_RUNTHREAD and IRQ_MASKED and wake
irq thread up and then the irq thread loops for another time.
So irq will not return unmask in case of oneshot.

If I miss something, correct me please.

Thanks,
Yong