Re: [Xen-devel] [PATCH v2] xen/arm: fix smpboot barriers

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 07/12/2016 19:02, Stefano Stabellini wrote:
> Remove useless smp_wmb() barrier after cpumask_set_cpu(cpuid,
> &cpu_online_map), which is not synchronizing against anything.
>
> Keep the other smp_wmb(), before the cpumask_set_cpu call, to ensure
> that all writes before setting the cpu online are visible to other cpus.
> For that to work properly, we need a corresponding smp_rmb() barrier,
> after reading the online cpumask from other processors, which is
> currently missing. Add it.
>
> See: http://marc.info/?l=xen-devel&m=148093236307211
>
> Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
With some minor coding styles change:

Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

Also, I am wondering whether we should backport this patch? Technically 
the barrier are wrong.
> ---
>
> Changes in v2:
> - add in-code comments
>
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 90ad1d0..4570f45 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -307,11 +307,12 @@ void start_secondary(unsigned long boot_phys_offset,
>
>      /* Run local notifiers */
>      notify_cpu_starting(cpuid);
> +    /* Ensure that previous writes are visible before marking the cpu as
> +     * online. */
It looks like smpboot.c is using different style for comment. However, 
the correct one is
/*
 * Ensure...
 * ...
 */

>      smp_wmb();
>
>      /* Now report this CPU is up */
>      cpumask_set_cpu(cpuid, &cpu_online_map);
> -    smp_wmb();
>
>      local_irq_enable();
>      local_abort_enable();
> @@ -408,6 +409,9 @@ int __cpu_up(unsigned int cpu)
>          cpu_relax();
>          process_pending_softirqs();
>      }
> +    /* Ensure that other cpus' initializations are visible before
> +     * proceeding. Corresponds to smp_wmb() in start_secondary. */
Ditto.

> +    smp_rmb();
>
>      /*
>       * Nuke start of day info before checking one last time if the CPU
--
Julien Grall

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