Re: [Xen-devel] [PATCH v1 2/2] xen/arm: Fix deadlock in on_selected_cpus function

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 27 Jan 2014, Oleksandr Tyshchenko wrote:
> This patch is needed to avoid possible deadlocks in case of simultaneous
> occurrence cross-interrupts.
> 
> Change-Id: I574b496442253a7b67a27e2edd793526c8131284
> Signed-off-by: Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>
> ---
>  xen/common/smp.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/smp.c b/xen/common/smp.c
> index 2700bd7..46d2fc6 100644
> --- a/xen/common/smp.c
> +++ b/xen/common/smp.c
> @@ -55,7 +55,11 @@ void on_selected_cpus(
>  
>      ASSERT(local_irq_is_enabled());
>  
> -    spin_lock(&call_lock);
> +    if (!spin_trylock(&call_lock)) {
> +        if (smp_call_function_interrupt())
> +            return;

If smp_call_function_interrupt returns -EPERM, shouldn't we go back to
spinning on call_lock?
Also there is a race condition between spin_lock, cpumask_copy and
smp_call_function_interrupt: smp_call_function_interrupt could be called
on cpu1 after cpu0 acquired the lock, but before cpu0 set
call_data.selected.

I think the correct implemention would be:


while ( unlikely(!spin_trylock(&call_lock)) )
    smp_call_function_interrupt();



> +        spin_lock(&call_lock);
> +    }
>  
>      cpumask_copy(&call_data.selected, selected);
>  
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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