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. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
