[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 01/49] xen/sched: call cpu_disable_scheduler() via cpu notifier



Hi Juergen,

On 4/1/19 11:37 AM, Juergen Gross wrote:
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...

Hmmm... looking at the other usage (such as in do_poll), on non-x86 platform, there is a smp_mb() between set_bit(...) and checking the event with a similar comment above.

I don't know enough the scheduler code to know why the barrier is needed. But for consistency, it seems to me the smp_mb() would be required in vcpu_block() as well.

Also, it is quite interesting that the barrier is not presence for x86. If I understand correctly the comment on top of set_bit/clear_bit, it could as well be re-ordered. So we seem to relying on the underlying implementation of set_bit/clear_bit.

Wouldn't it make sense to try to uniformize the semantics? Maybe by introducing a new helper?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.