Messages in this thread | | | Date | Tue, 25 Oct 2022 16:55:47 +0100 | Subject | Re: [RFD] resctrl: reassigning a running container's CTRL_MON group | From | James Morse <> |
| |
Hi Peter,
On 21/10/2022 13:42, Peter Newman wrote: > On Thu, Oct 20, 2022 at 12:39 PM Peter Newman <peternewman@google.com> wrote: >> >> On Wed, Oct 19, 2022 at 3:58 PM James Morse <james.morse@arm.com> wrote: >>> The devil is in the detail, I'm not sure how it serialises with a fork()ing process, I'd >>> hope to do better than relying on the kernel walking the list of processes a lot quicker >>> than user-space can. >> >> I wasn't planning to do it any more optimally than the rmdir >> implementation today when looking for all tasks impacted by a >> CLOSID/RMID deletion. > > This is probably a separate topic, but I noticed this when looking at how rmdir > moves tasks to a new closid/rmid... > > In rdt_move_group_tasks(), how do we know that a task switching in on another > CPU will observe the updated closid and rmid values soon enough? > > 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?
Thanks,
James
| |