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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.04.14 at 18:23, <julien.grall@xxxxxxxxxx> wrote:
> 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.

That would set you up for problems with the handler wanting to
manipulate its IRQ (which might imply locking desc). I'd suggest
looking at how Linux deals with this (synchronize_irq() in particular).

Jan


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