Re: [Xen-devel] [PATCH v3 18/18] xen/arm: IRQ: Handle multiple action per IRQ

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote:
> On 04/16/2014 04:54 PM, Ian Campbell wrote:
> > On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote:
> >>          desc->status &= ~IRQ_PENDING;
> >>          spin_unlock_irq(&desc->lock);
> >> -        action->handler(irq, action->dev_id, regs);
> >> +        list_for_each_entry_safe(action, next, &desc->action, next)
> >> +            action->handler(irq, action->dev_id, regs);
> > 
> > You aren't removing entries from within the loop so I don't think you
> > need the _safe variant.
> 
> As we release the desc->lock here, it might be possible to have the list
> changed under the CPU feet by release_irq.
> 
> With the double-linked list, how do we make sure that it won't happen?

Normally by using a lock. I don't know if even list_for_each_entry_safe
is safe against concurrent changes to the list from other threads, I
think it only refers to safe to changing the list within the loop in
this thread.
 
> > I actually think a goto found would actually be clearer here than the
> > flag.
> 
> I'm not a big fan of goto.
> 
> Anyway, I will use it here if you think it's clearer.

It has it's places, I think this is one of them.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel