Re: [PATCH 1/2] xen: allow on_selected_cpus with interrupts disabled

[Keir Fraser <keir.xen@gmail.com>]

On 03/05/2013 16:21, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

> On Fri, 2013-05-03 at 15:58 +0100, Stefano Stabellini wrote:
>> Allow on_selected_cpus with interrupts disabled, use it with care.
> 
> This is a deadlock waiting to happen. Can we not find a way to do cross
> CPU EOI without it? If we can guarantee that we only need to EOI on one
> CPU then does that make a specialised SGI vector more plausible?
> 
> Can the IPI call not be moved outside the lock? i.e. remove it from the
> list under the lock and then IPI outside?
> 
> Or could you queue the IRQ on a per-pcpu list of IRQs to EOI and then
> outside the lock send an IPI to the other CPU to check the list.
> 
> At the least this should assert that he current cpu isn't in the mask
> when wait == 1.

There's little chance of me being flexible on changing the
on_selected_cpus() interface. This may be better handled under arch/arm, or
with a new interface, or just as you sugegst rethinking the higher-level
problem so you don't get painted into this corner in the first place.
 
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> ---
>>  xen/common/smp.c |    7 +++----
>>  1 files changed, 3 insertions(+), 4 deletions(-)
>> 
>> diff --git a/xen/common/smp.c b/xen/common/smp.c
>> index dcd93ad..7deb97c 100644
>> --- a/xen/common/smp.c
>> +++ b/xen/common/smp.c
>> @@ -33,10 +33,9 @@ void on_selected_cpus(
>>      int wait)
>>  {
>>      unsigned int nr_cpus;
>> +    unsigned long flags;
>>  
>> -    ASSERT(local_irq_is_enabled());
>> -
>> -    spin_lock(&call_lock);
>> +    spin_lock_irqsave(&call_lock, flags);
>>  
>>      cpumask_copy(&call_data.selected, selected);
>>  
>> @@ -54,7 +53,7 @@ void on_selected_cpus(
>>          cpu_relax();
>>  
>>  out:
>> -    spin_unlock(&call_lock);
>> +    spin_unlock_irqrestore(&call_lock, flags);
>>  }
>>  
>>  void smp_call_function_interrupt(void)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel