On 01/04/2019 12:29, Julien Grall wrote: > Hi, > > On 4/1/19 10:40 AM, Juergen Gross wrote: >> On 01/04/2019 11:21, Julien Grall wrote: >>> Hi, >>> >>> On 3/29/19 3:08 PM, Juergen Gross wrote: >>>> cpu_disable_scheduler() is being called from __cpu_disable() today. >>>> There is no need to execute it on the cpu just being disabled, so use >>>> the CPU_DEAD case of the cpu notifier chain. Moving the call out of >>>> stop_machine() context is fine, as we just need to hold the domain RCU >>>> lock and need the scheduler percpu data to be still allocated. >>>> >>>> Add another hook for CPU_DOWN_PREPARE to bail out early in case >>>> cpu_disable_scheduler() would fail. This will avoid crashes in rare >>>> cases for cpu hotplug or suspend. >>>> >>>> While at it remove a superfluous smp_mb() in the ARM __cpu_disable() >>>> incarnation. >>> >>> This is not obvious why the smp_mb() is superfluous. Can you please >>> provide more details on why this is not necessary? >> >> cpumask_clear_cpu() should already have the needed semantics, no? >> It is based on clear_bit() which is defined to be atomic. > > atomicity does not mean the store/load cannot be re-ordered by the CPU. > You would need a barrier to prevent re-ordering. > > cpumask_clear_cpu() and clear_bit() does not contain any barrier, so > store/load can be re-ordered.
Uh, couldn't this lead to problems, e.g. in vcpu_block()? The comment there suggests the sequence of setting the blocked bit and doing the test is important for avoiding a race... Juergen > > I see we have similar smp_mb() barrier in __cpu_die(). Sadly, there are > no documentation in the code why the barrier is here. The logs don't > help either. > > The barrier here will ensure that the load/store related to disabling > the CPU are seen before any load/store happening after the return. > Although, I am not sure why this is necessary. > > Stefano, Do you remember the rationale? > > Cheers, > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
