On Thu, Jul 06, 2023 at 12:23:57AM +0200, Frederic Weisbecker wrote:
> Le Wed, Jul 05, 2023 at 07:12:53PM +0100, Valentin Schneider a écrit :
> +bool ct_set_cpu_work(unsigned int cpu, unsigned int work)
> +{
> + struct context_tracking *ct = per_cpu_ptr(&context_tracking, cpu);
> + unsigned int old, new, state;
> + bool ret = false;
> +
> + preempt_disable();
> +
> + work <<= CONTEXT_WORK;
> + state = atomic_read(&ct->state);
> + /*
> + * Try setting the work until either
> + * - the target CPU is on the kernel
> + * - the work has been set
> + */
> + for (;;) {
> + /* Only set if running in user/guest */
> + old = state;
> + old &= ~CONTEXT_MASK;
> + old |= CONTEXT_USER;
> +
> + new = old | work;
> +
> + state = atomic_cmpxchg(&ct->state, old, new);
> + if (state & work) {
And this should be "if (state == old)", otherwise there is
a risk that someone else had set the work but atomic_cmpxchg()
failed due to other modifications is the meantime. It's then
dangerous in that case to defer the work because atomic_cmpxchg()
failures don't imply full ordering. So there is a risk that the
target executes the work but doesn't see the most recent data.
Thanks.
