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

[Julien Grall <julien.grall@linaro.org>]

On 04/16/2014 05:17 PM, Ian Campbell wrote:
> 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.
>  

Hmmmm... I'm wondering if we can keep desc->lock held while calling the
action handler and enable IRQ.

For SPI, we are fine as the same SPI can't be fired twice.

For PPI, it's bank so it's fine.

Did I miss something?

-- 
Julien Grall