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

Re: [PATCH for-4.15] xen/sched: Add missing memory barrier in vcpu_block()


On 23/02/2021 07:00, Jan Beulich wrote:
On 22.02.2021 21:12, Julien Grall wrote:
On 22/02/2021 20:09, Stefano Stabellini wrote:
On Mon, 22 Feb 2021, Jan Beulich wrote:
On 20.02.2021 20:47, Julien Grall wrote:
This is a follow-up of the discussion that started in 2019 (see [1])
regarding a possible race between do_poll()/vcpu_unblock() and the wake
up path.

I haven't yet fully thought about the potential race in do_poll(). If
there is, then this would likely want to be fixed in a separate patch.

On x86, the current code is safe because set_bit() is fully ordered. SO
the problem is Arm (and potentially any new architectures).

I couldn't convince myself whether the Arm implementation of
local_events_need_delivery() contains enough barrier to prevent the
re-ordering. However, I don't think we want to play with devil here as
the function may be optimized in the future.

In fact I think this ...

--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -1418,6 +1418,8 @@ void vcpu_block(void)
set_bit(_VPF_blocked, &v->pause_flags); + smp_mb__after_atomic();

... pattern should be looked for throughout the codebase, and barriers
be added unless it can be proven none is needed. >
And in that case it would be best to add an in-code comment to explain
why the barrier is not needed
I would rather not add comment for every *_bit() calls. It should be
pretty obvious for most of them that the barrier is not necessary.

We should only add comments where the barrier is necessary or it is not
clear why it is not necessary.

I guess by "pattern" I didn't necessarily mean _all_ *_bit()
calls - indeed there are many where it's clear that no barrier
would be needed. I was rather meaning modifications like this
of v->pause_flags (I'm sure there are further such fields).

Agree, this work is mostly a side project for now. So I will continue to go through the pattern when I find time.


Julien Grall



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