Re: [PATCH 06/10] xen: arm: Use SMP barriers when that is all which is required.

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2013-07-01 at 16:19 +0100, Stefano Stabellini wrote:
> On Fri, 28 Jun 2013, Ian Campbell wrote:
> > As explained in the previous commit SMP barriers can be used when all we care
> > about is synchronising against other processors.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/mm.c      |    2 +-
> >  xen/arch/arm/smpboot.c |    4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index c5213f2..3f049cb 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -776,7 +776,7 @@ void share_xen_page_with_guest(struct page_info *page,
> >      page->u.inuse.type_info |= PGT_validated | 1;
> >  
> >      page_set_owner(page, d);
> > -    wmb(); /* install valid domain ptr before updating refcnt. */
> > +    smp_wmb(); /* install valid domain ptr before updating refcnt. */
> >      ASSERT((page->count_info & ~PGC_xen_heap) == 0);
> >  
> >      /* Only add to the allocation list if the domain isn't dying. */
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index 8011987..727e09f 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c
> > @@ -170,11 +170,11 @@ void __cpuinit start_secondary(unsigned long boot_phys_offset,
> >  
> >      /* Run local notifiers */
> >      notify_cpu_starting(cpuid);
> > -    wmb();
> > +    smp_wmb();
> >  
> >      /* Now report this CPU is up */
> >      cpumask_set_cpu(cpuid, &cpu_online_map);
> > -    wmb();
> > +    smp_wmb();
> >  
> >      local_irq_enable();
> 
> Did you missed few mb() in smpboot.c?

The ones in __cpu_disable and __cpu_die?

I think I just wasn't 100% sure they might not be touching hardware
(i.e. some platform register to shutdown a CPU) and since they weren't
performance critical I punted on them.

Looking it again the first half of that logic seems to be bogus (that
code goes nowhere near any peripheral).

Ian.