Received-SPF: pass (google.com: domain of linux-kernel+bounces-117602-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 25 Mar 2024 10:27:07 -0700
In-Reply-To: <20240325172707.73966-1-peternewman@google.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240325172707.73966-1-peternewman@google.com>
Message-ID: <20240325172707.73966-7-peternewman@google.com>
Subject: [PATCH v1 6/6] x86/resctrl: Don't search tasklist in mongroup rename
From: Peter Newman <peternewman@google.com>
To: Fenghua Yu <fenghua.yu@intel.com>, Reinette Chatre <reinette.chatre@intel.com>, 
	James Morse <james.morse@arm.com>
Cc: Stephane Eranian <eranian@google.com>, Thomas Gleixner <tglx@linutronix.de>, 
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, 
	"H. Peter Anvin" <hpa@zytor.com>, Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, 
	Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, 
	Steven Rostedt <rostedt@goodmis.org>, Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, 
	Daniel Bristot de Oliveira <bristot@redhat.com>, Valentin Schneider <vschneid@redhat.com>, 
	Uros Bizjak <ubizjak@gmail.com>, Mike Rapoport <rppt@kernel.org>, 
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, 
	Rick Edgecombe <rick.p.edgecombe@intel.com>, Xin Li <xin3.li@intel.com>, 
	Babu Moger <babu.moger@amd.com>, Shaopeng Tan <tan.shaopeng@fujitsu.com>, 
	Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>, Jens Axboe <axboe@kernel.dk>, 
	Christian Brauner <brauner@kernel.org>, Oleg Nesterov <oleg@redhat.com>, 
	Andrew Morton <akpm@linux-foundation.org>, Tycho Andersen <tandersen@netflix.com>, 
	Nicholas Piggin <npiggin@gmail.com>, Beau Belgrave <beaub@linux.microsoft.com>, 
	"Matthew Wilcox (Oracle)" <willy@infradead.org>, linux-kernel@vger.kernel.org, 
	Peter Newman <peternewman@google.com>
Content-Type: text/plain; charset="UTF-8"

Iterating over all task_structs while read-locking the tasklist_lock
results in significant task creation/destruction latency. Back-to-back
move operations can thus be disastrous to the responsiveness of
threadpool-based services.

Now that the CLOSID is determined indirectly through a reference to the
task's current rdtgroup, it is not longer necessary to update the CLOSID
in all tasks belonging to the moved mongroup. The context switch handler
just needs to be prepared for concurrent writes to the parent pointer.

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

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index bd067f7ed5b6..a007c0ec478f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -388,8 +388,11 @@ void __resctrl_sched_in(struct task_struct *tsk)
 				 * by a full barrier and synchronous IPI
 				 * broadcast before proceeding to free the
 				 * group.
+				 *
+				 * parent can be concurrently updated to a new
+				 * group as a result of mongrp_reparent().
 				 */
-				closid = rgrp->mon.parent->closid;
+				closid = READ_ONCE(rgrp->mon.parent)->closid;
 		} else {
 			closid = rgrp->closid;
 		}
@@ -3809,8 +3812,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
  * Monitoring data for the group is unaffected by this operation.
  */
 static void mongrp_reparent(struct rdtgroup *rdtgrp,
-			    struct rdtgroup *new_prdtgrp,
-			    cpumask_var_t cpus)
+			    struct rdtgroup *new_prdtgrp)
 {
 	struct rdtgroup *prdtgrp = rdtgrp->mon.parent;
 
@@ -3825,13 +3827,10 @@ static void mongrp_reparent(struct rdtgroup *rdtgrp,
 	list_move_tail(&rdtgrp->mon.crdtgrp_list,
 		       &new_prdtgrp->mon.crdtgrp_list);
 
-	rdtgrp->mon.parent = new_prdtgrp;
+	WRITE_ONCE(rdtgrp->mon.parent, new_prdtgrp);
 	rdtgrp->closid = new_prdtgrp->closid;
 
-	/* Propagate updated closid to all tasks in this group. */
-	rdt_move_group_tasks(rdtgrp, rdtgrp, cpus);
-
-	update_closid_rmid(cpus, NULL);
+	update_closid_rmid(cpu_online_mask, NULL);
 }
 
 static int rdtgroup_rename(struct kernfs_node *kn,
@@ -3839,7 +3838,6 @@ static int rdtgroup_rename(struct kernfs_node *kn,
 {
 	struct rdtgroup *new_prdtgrp;
 	struct rdtgroup *rdtgrp;
-	cpumask_var_t tmpmask;
 	int ret;
 
 	rdtgrp = kernfs_to_rdtgroup(kn);
@@ -3909,16 +3907,6 @@ static int rdtgroup_rename(struct kernfs_node *kn,
 		goto out;
 	}
 
-	/*
-	 * Allocate the cpumask for use in mongrp_reparent() to avoid the
-	 * possibility of failing to allocate it after kernfs_rename() has
-	 * succeeded.
-	 */
-	if (!zalloc_cpumask_var(&tmpmask, GFP_KERNEL)) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
 	/*
 	 * Perform all input validation and allocations needed to ensure
 	 * mongrp_reparent() will succeed before calling kernfs_rename(),
@@ -3927,9 +3915,7 @@ static int rdtgroup_rename(struct kernfs_node *kn,
 	 */
 	ret = kernfs_rename(kn, new_parent, new_name);
 	if (!ret)
-		mongrp_reparent(rdtgrp, new_prdtgrp, tmpmask);
-
-	free_cpumask_var(tmpmask);
+		mongrp_reparent(rdtgrp, new_prdtgrp);
 
 out:
 	mutex_unlock(&rdtgroup_mutex);
-- 
2.44.0.396.g6e790dbe36-goog