Messages in this thread Patch in this message | | | Date | Tue, 29 Nov 2022 12:10:54 +0100 | Subject | [PATCH v4 1/2] x86/resctrl: Update task closid/rmid with task_call_func() | From | Peter Newman <> |
| |
When the user moves a running task to a new rdtgroup using the tasks file interface, the resulting change in CLOSID/RMID must be immediately propagated to the PQR_ASSOC MSR on the task's CPU.
It is possible for a task to wake up or migrate while it is being moved to a new group. If __rdtgroup_move_task() fails to observe that a task has begun running or misses that it migrated to a new CPU, the task will continue to use the old CLOSID or RMID until it switches in again.
__rdtgroup_move_task() assumes that if the task migrates off of its CPU before it can IPI the task, then the task has already observed the updated CLOSID/RMID. Because this is done locklessly and an x86 CPU can delay stores until after loads, the following incorrect scenarios are possible:
1. __rdtgroup_move_task() stores the new closid and rmid in the task structure after it loads task_curr() and task_cpu(). 2. resctrl_sched_in() loads t->{closid,rmid} before the calling context switch stores new task_curr() and task_cpu() values.
Use task_call_func() in __rdtgroup_move_task() to serialize updates to the closid and rmid fields in the task_struct with context switch.
Signed-off-by: Peter Newman <peternewman@google.com> Reviewed-by: James Morse <james.morse@arm.com> --- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 78 ++++++++++++++++---------- 1 file changed, 47 insertions(+), 31 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index e5a48f05e787..59b7ffcd53bb 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -528,6 +528,31 @@ static void rdtgroup_remove(struct rdtgroup *rdtgrp) kfree(rdtgrp); } +static int update_locked_task_closid_rmid(struct task_struct *t, void *arg) +{ + struct rdtgroup *rdtgrp = arg; + + /* + * Although task_call_func() serializes the writes below with the paired + * reads in resctrl_sched_in(), resctrl_sched_in() still needs + * READ_ONCE() due to rdt_move_group_tasks(), so use WRITE_ONCE() here + * to conform. + */ + if (rdtgrp->type == RDTCTRL_GROUP) { + WRITE_ONCE(t->closid, rdtgrp->closid); + WRITE_ONCE(t->rmid, rdtgrp->mon.rmid); + } else if (rdtgrp->type == RDTMON_GROUP) { + WRITE_ONCE(t->rmid, rdtgrp->mon.rmid); + } + + /* + * If the task is current on a CPU, the PQR_ASSOC MSR needs to be + * updated to make the resource group go into effect. If the task is not + * current, the MSR will be updated when the task is scheduled in. + */ + return task_curr(t); +} + static void _update_task_closid_rmid(void *task) { /* @@ -538,10 +563,24 @@ static void _update_task_closid_rmid(void *task) resctrl_sched_in(); } -static void update_task_closid_rmid(struct task_struct *t) +static void update_task_closid_rmid(struct task_struct *t, + struct rdtgroup *rdtgrp) { - if (IS_ENABLED(CONFIG_SMP) && task_curr(t)) - smp_call_function_single(task_cpu(t), _update_task_closid_rmid, t, 1); + /* + * Serialize the closid and rmid update with context switch. If + * task_call_func() indicates that the task was running during + * update_locked_task_closid_rmid(), then interrupt it. + */ + if (task_call_func(t, update_locked_task_closid_rmid, rdtgrp) && + IS_ENABLED(CONFIG_SMP)) + /* + * If the task has migrated away from the CPU indicated by + * task_cpu() below, then it has already switched in on the + * new CPU using the updated closid and rmid and the call below + * is unnecessary, but harmless. + */ + smp_call_function_single(task_cpu(t), + _update_task_closid_rmid, t, 1); else _update_task_closid_rmid(t); } @@ -557,39 +596,16 @@ static int __rdtgroup_move_task(struct task_struct *tsk, return 0; /* - * Set the task's closid/rmid before the PQR_ASSOC MSR can be - * updated by them. - * - * For ctrl_mon groups, move both closid and rmid. * For monitor groups, can move the tasks only from * their parent CTRL group. */ - - if (rdtgrp->type == RDTCTRL_GROUP) { - WRITE_ONCE(tsk->closid, rdtgrp->closid); - WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid); - } else if (rdtgrp->type == RDTMON_GROUP) { - if (rdtgrp->mon.parent->closid == tsk->closid) { - WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid); - } else { - rdt_last_cmd_puts("Can't move task to different control group\n"); - return -EINVAL; - } + if (rdtgrp->type == RDTMON_GROUP && + rdtgrp->mon.parent->closid != tsk->closid) { + rdt_last_cmd_puts("Can't move task to different control group\n"); + return -EINVAL; } - /* - * Ensure the task's closid and rmid are written before determining if - * the task is current that will decide if it will be interrupted. - */ - barrier(); - - /* - * By now, the task's closid and rmid are set. If the task is current - * on a CPU, the PQR_ASSOC MSR needs to be updated to make the resource - * group go into effect. If the task is not current, the MSR will be - * updated when the task is scheduled in. - */ - update_task_closid_rmid(tsk); + update_task_closid_rmid(tsk, rdtgrp); return 0; } -- 2.38.1.584.g0f3c55d4c2-goog
| |