From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1DE252D8799 for ; Wed, 10 Dec 2025 16:51:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765385515; cv=none; b=lKPX/j0d86WeIInN1QnkrxVR2/3G9KgqnZ6EgRhHlFAjt7oSLJaEvUzkq6TXHzxe69X9+UeZ2Z85aBab39SFlBbq5bbKqq4VULKDsAMUwknAiNGg8d17D6DBEFY0T7LFXKJekZfG+m3D5aGDRdtgynxRAT/nun6XB3KWsgNSpsg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765385515; c=relaxed/simple; bh=nbHWFh6M+8/nbcibOo/uOlBJ4HCbYO4NMLzCRxY1SAE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SY/jmgoa/GOco9fhBLPUj+mwAKvqVQDYvV4dl8znwAdTRP710pba4Ue8aYYzThcOx3oP2woOgiu9Jji2spiV/+UFqtlXbKvSLTC4dtFZ6Srs4dhmqVYQk05h5b4GWK9UKCebBzrkdm7FXXt4AV4DxFRAFT3TuHjzGOGvmR4vdCw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=NyCqHveQ; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="NyCqHveQ" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=FXZTuGTZhC8tcGMUZPDBG+n/Ei6trFY83GzChzq8dXA=; b=NyCqHveQDEwjw2lR3O7mDsjIsm lUoA4BZkWlb+iTufe0zHsx1LUwTS+E6VVuVU5wVEAmUDawFl4PED+0Xb0/H38sZpn2x2I75HtzDlD PSP8juZv2/D0qYwn/TR8KFkg8R+lgImqwYrjI8CYKDatS5S8Rkp6dDLe/c63Z7W/066fimT48ny0S xBuRb1zi6QXkSPVfDDgNSSJECIcj0RSQtfVJim0Pgvo/MN7PCevw9evwCbUUSSgLdTGu9f7mzWLjf PZAM5/tLeImN40h9Mt8JJ/uga9MKnlCW830Ksn2o9SAHWuCoUgZW+QM8QkTRltNpSUnYASW7aA7C8 8wZbMivQ==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.98.2 #2 (Red Hat Linux)) id 1vTNPM-0000000D6OG-18aJ; Wed, 10 Dec 2025 16:51:20 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id D2DD0300566; Wed, 10 Dec 2025 17:51:19 +0100 (CET) Date: Wed, 10 Dec 2025 17:51:19 +0100 From: Peter Zijlstra To: Tim Chen Cc: Ingo Molnar , K Prateek Nayak , "Gautham R . Shenoy" , Vincent Guittot , Chen Yu , Juri Lelli , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Madadi Vineeth Reddy , Hillf Danton , Shrikanth Hegde , Jianyong Wu , Yangyu Chen , Tingyin Duan , Vern Hao , Vern Hao , Len Brown , Aubrey Li , Zhao Liu , Chen Yu , Adam Li , Aaron Lu , Tim Chen , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 17/23] sched/cache: Record the number of active threads per process for cache-aware scheduling Message-ID: <20251210165119.GY3707891@noisy.programming.kicks-ass.net> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, Dec 03, 2025 at 03:07:36PM -0800, Tim Chen wrote: > From: Chen Yu > > A performance regression was observed by Prateek when running hackbench > with many threads per process (high fd count). To avoid this, processes > with a large number of active threads are excluded from cache-aware > scheduling. > > With sched_cache enabled, record the number of active threads in each > process during the periodic task_cache_work(). While iterating over > CPUs, if the currently running task belongs to the same process as the > task that launched task_cache_work(), increment the active thread count. > > This number will be used by subsequent patch to inhibit cache aware > load balance. > > Suggested-by: K Prateek Nayak > Signed-off-by: Chen Yu > Signed-off-by: Tim Chen > --- > > Notes: > v1->v2: No change. > > include/linux/mm_types.h | 1 + > kernel/sched/fair.c | 11 +++++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 1ea16ef90566..04743983de4d 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -1043,6 +1043,7 @@ struct mm_struct { > raw_spinlock_t mm_sched_lock; > unsigned long mm_sched_epoch; > int mm_sched_cpu; > + u64 nr_running_avg ____cacheline_aligned_in_smp; This is unlikely to do what you hope it does, it will place this variable on a new cacheline, but will not ensure this variable is the only one in that line. Notably ogtables_bytes (the next field in this structure) will share the line. It might all be less dodgy if you stick these here fields in their own structure, a little like mm_mm_cid or so. > #endif > > #ifdef CONFIG_MMU > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 580a967efdac..2f38ad82688f 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -1421,11 +1421,11 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) > > static void __no_profile task_cache_work(struct callback_head *work) > { > - struct task_struct *p = current; > + struct task_struct *p = current, *cur; > struct mm_struct *mm = p->mm; > unsigned long m_a_occ = 0; > unsigned long curr_m_a_occ = 0; > - int cpu, m_a_cpu = -1; > + int cpu, m_a_cpu = -1, nr_running = 0; > cpumask_var_t cpus; > > WARN_ON_ONCE(work != &p->cache_work); > @@ -1458,6 +1458,12 @@ static void __no_profile task_cache_work(struct callback_head *work) > m_occ = occ; > m_cpu = i; > } guard(rcu)(); > + rcu_read_lock(); > + cur = rcu_dereference(cpu_rq(i)->curr); > + if (cur && !(cur->flags & (PF_EXITING | PF_KTHREAD)) && > + cur->mm == mm) > + nr_running++; > + rcu_read_unlock(); > } > > /* > @@ -1501,6 +1507,7 @@ static void __no_profile task_cache_work(struct callback_head *work) > mm->mm_sched_cpu = m_a_cpu; > } > > + update_avg(&mm->nr_running_avg, nr_running); > free_cpumask_var(cpus); > } Its a wee bit weird to introduce nr_running_avg without its user. Makes it hard to see what's what.