>>> On 05.12.16 at 14:59, <andrew.coop...@citrix.com> wrote: > On 05/12/16 13:50, Jan Beulich wrote: >>>>> On 05.12.16 at 14:43, <andrew.coop...@citrix.com> wrote: >>> On 05/12/16 12:28, Jan Beulich wrote: >>>>>>> On 05.12.16 at 12:25, <andrew.coop...@citrix.com> wrote: >>>>> On 05/12/16 11:18, Jan Beulich wrote: >>>>>>>>> On 05.12.16 at 11:05, <andrew.coop...@citrix.com> wrote: >>>>>>> --- a/xen/arch/x86/smpboot.c >>>>>>> +++ b/xen/arch/x86/smpboot.c >>>>>>> @@ -346,7 +346,6 @@ void start_secondary(void *unused) >>>>>>> spin_debug_enable(); >>>>>>> set_cpu_sibling_map(cpu); >>>>>>> notify_cpu_starting(cpu); >>>>>>> - wmb(); >>>>>>> >>>>>>> /* >>>>>>> * We need to hold vector_lock so there the set of online cpus >>>>>> Hmm, this one is indeed redundant with the lock_vector_lock() >>>>>> following right below, but if that lock wasn't there, I think it >>>>>> would be needed to order set_cpu_sibling_map() and the >>>>>> setting of the bit in the online map. So I think it would better >>>>>> stay (but be relaxed to smb_wmb()). >>>>> Why? It doesn't relate to an smp_rmb() on any other CPU, and is >>>>> therefore wrong to use. >>>> I think it does, just not with one that's spelled out as smp_rmb(). >>>> Instead __cpu_up() spins on !cpu_online(), using cpu_relax() as >>>> a de-facto equivalent of smp_rmb(). >>> __cpu_up() is ordered with the cpumask_set_cpu(cpu, &cpu_online_map) >>> between the two context hunks. >> Exactly - so here we need the write side to that > > No, we don't. > > cpumask_set_cpu(cpu, &cpu_online_map) is a write operation, so orders > properly on x86. C's ordering properties ensure that the adjacent > function calls happen in program order.
Well, that then again falls into the category of barriers which would be needed in arch-independent code, but can be omitted in x86-specific sources. I think we should separate both classes when relaxing/eliminating them. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
