lkml.org 
[lkml]   [2022]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
Subject[PATCH v3 2/2] x86/resctrl: update task closid/rmid with task_call_func()
From
When determining whether running tasks need to be interrupted due to a
closid/rmid change, it was possible for the task in question to migrate
or wake up concurrently without observing the updated values.

In particular, __rdtgroup_move_task() assumed that a CPU migrating
implied that it observed the updated closid/rmid. This assumption is
broken by the following reorderings, both of which are allowed on x86:

1. In __rdtgroup_move_task(), stores updating the closid and rmid in
the task structure could reorder with the loads in task_curr() and
task_cpu().
2. In resctrl_sched_in(), the earlier stores to the fields read by
task_curr() could be delayed until after the loads from
t->{closid,rmid}.

Preventing this reordering with barriers would have required an smp_mb()
in all context switches whenever resctrlfs is mounted. Instead, when
moving a single task, use task_call_func() 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 | 76 +++++++++++++++-----------
1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 049971efea2f..511b7cea143f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -538,10 +538,47 @@ static void _update_task_closid_rmid(void *task)
resctrl_sched_in();
}

-static void update_task_closid_rmid(struct task_struct *t)
+static int update_locked_task_closid_rmid(struct task_struct *t, void *arg)
{
- if (IS_ENABLED(CONFIG_SMP) && task_curr(t))
- smp_call_function_single(task_cpu(t), _update_task_closid_rmid, t, 1);
+ struct rdtgroup *rdtgrp = arg;
+
+ /*
+ * We assume task_call_func() has provided the necessary serialization
+ * with resctrl_sched_in().
+ */
+ if (rdtgrp->type == RDTCTRL_GROUP) {
+ t->closid = rdtgrp->closid;
+ t->rmid = rdtgrp->mon.rmid;
+ } else if (rdtgrp->type == RDTMON_GROUP) {
+ 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(struct task_struct *t,
+ struct rdtgroup *rdtgrp)
+{
+ /*
+ * Serialize the closid and rmid update with context switch. If this
+ * function indicates that the task was running, then it needs to be
+ * interrupted to install the new closid and rmid.
+ */
+ 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
+ * unnecessary, but harmless.
+ */
+ smp_call_function_single(task_cpu(t),
+ _update_task_closid_rmid, t, 1);
else
_update_task_closid_rmid(t);
}
@@ -557,39 +594,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.493.g58b659f92b-goog
\
 
 \ /
  Last update: 2022-11-15 15:22    [W:0.143 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site