Re: [RFD] resctrl: reassigning a running container's CTRL_MON group

[Peter Newman <peternewman@google.com>]

On Tue, Oct 25, 2022 at 5:55 PM James Morse <james.morse@arm.com> wrote:
> On 21/10/2022 13:42, Peter Newman wrote:
> > Even on x86, without an smp_mb(), the stores to t->closid and t->rmid could be
> > reordered with the task_curr(t) and task_cpu(t) reads which follow. The original
> > description of this scenario seemed to assume that accesses below would happen
> > in program order:
> >
> >     WRITE_ONCE(t->closid, to->closid);
> >     WRITE_ONCE(t->rmid, to->mon.rmid);
> >
> >     /*
> >      * If the task is on a CPU, set the CPU in the mask.
> >      * The detection is inaccurate as tasks might move or
> >      * schedule before the smp function call takes place.
> >      * In such a case the function call is pointless, but
> >      * there is no other side effect.
> >      */
> >     if (IS_ENABLED(CONFIG_SMP) && mask && task_curr(t))
> >          cpumask_set_cpu(task_cpu(t), mask);
> >
> > If the task concurrently switches in on another CPU, the code above may not
> > observed that it's running, and the CPU running the task may not have observed
> > the updated rmid and closid yet, so it could continue with the old rmid/closid
> > and not get interrupted.
>
> Makes sense to me - do you want to send a patch to fix it?

Sure, when I think of a solution. For an smp_mb() to be effective above,
we would need to execute another smp_mb() unconditionally before reading
the closid/rmid fields when switching a task in.

The only quick fix I know will work without badly hurting context switch
time would be to go back to pinging all CPUs following a mass
task-movement operation.

I'll see if I can come up with anything better, though.

-Peter