Date:   Wed, 25 Jan 2023 11:13:33 +0100
In-Reply-To: <20230125101334.1069060-1-peternewman@google.com>
Mime-Version: 1.0
References: <20230125101334.1069060-1-peternewman@google.com>
Message-ID: <20230125101334.1069060-3-peternewman@google.com>
Subject: [PATCH v3 2/3] x86/resctrl: Factor work to update task CLOSID/RMID
From:   Peter Newman <peternewman@google.com>
To:     reinette.chatre@intel.com, fenghua.yu@intel.com
Cc:     Babu.Moger@amd.com, bp@alien8.de, dave.hansen@linux.intel.com,
        eranian@google.com, gupasani@google.com, hpa@zytor.com,
        james.morse@arm.com, linux-kernel@vger.kernel.org,
        mingo@redhat.com, skodak@google.com, tony.luck@intel.com,
        tglx@linutronix.de, x86@kernel.org,
        Peter Newman <peternewman@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Functions that update a task's CLOSID or RMID must determine whether the
task is concurrently running to determine whether the task needs to be
interrupted.

Negotiating the race conditions involved is nuanced, so spare new types
of task group-moving functionality from needing to understand the fine
details.

Factor the task_struct::{closid,rmid} update along with the synchronized
concurrently-running-task check from rdt_move_group_tasks() into a new
rdt_move_one_task() helper. Use this helper in __rdtgroup_move_task() as
well.

This should not result in any functional change.

Signed-off-by: Peter Newman <peternewman@google.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 60 +++++++++++++-------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index c3fb525d52e9..b2081bc1bbfd 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 void rdt_move_one_task(struct task_struct *t, u32 closid, u32 rmid,
+			      cpumask_var_t mask)
+{
+	WRITE_ONCE(t->closid, closid);
+	WRITE_ONCE(t->rmid, rmid);
+
+	/*
+	 * Order the closid/rmid stores above before the loads
+	 * in task_curr(). This pairs with the full barrier
+	 * between the rq->curr update and resctrl_sched_in()
+	 * during context switch.
+	 */
+	smp_mb();
+
+	/*
+	 * 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);
+}
+
 static void _update_task_closid_rmid(void *task)
 {
 	/*
@@ -566,25 +591,17 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
 	 */
 
 	if (rdtgrp->type == RDTCTRL_GROUP) {
-		WRITE_ONCE(tsk->closid, rdtgrp->closid);
-		WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid);
+		rdt_move_one_task(tsk, rdtgrp->closid, rdtgrp->mon.rmid, NULL);
 	} else if (rdtgrp->type == RDTMON_GROUP) {
 		if (rdtgrp->mon.parent->closid == tsk->closid) {
-			WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid);
+			rdt_move_one_task(tsk, tsk->closid, rdtgrp->mon.rmid,
+					  NULL);
 		} else {
 			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.
-	 * This pairs with the full barrier between the rq->curr update and
-	 * resctrl_sched_in() during context switch.
-	 */
-	smp_mb();
-
 	/*
 	 * 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
@@ -2409,26 +2426,7 @@ static void rdt_move_group_tasks(struct rdtgroup *from, struct rdtgroup *to,
 	for_each_process_thread(p, t) {
 		if (!from || is_closid_match(t, from) ||
 		    is_rmid_match(t, from)) {
-			WRITE_ONCE(t->closid, to->closid);
-			WRITE_ONCE(t->rmid, to->mon.rmid);
-
-			/*
-			 * Order the closid/rmid stores above before the loads
-			 * in task_curr(). This pairs with the full barrier
-			 * between the rq->curr update and resctrl_sched_in()
-			 * during context switch.
-			 */
-			smp_mb();
-
-			/*
-			 * 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);
+			rdt_move_one_task(t, to->closid, to->mon.rmid, mask);
 		}
 	}
 	read_unlock(&tasklist_lock);
-- 
2.39.1.405.gd4c25cc71f-goog