linux-kernel - Re: [RFC PATCH v4 05/28] sched: Add hysteresis to switch a task's preferred LLC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <11b8de67-ce3d-4c07-a5b7-1c8e1f58f71c@intel.com>
Date: Tue, 30 Sep 2025 12:31:28 +0800
From: "Chen, Yu C" <yu.c.chen@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: Ingo Molnar <mingo@...hat.com>, K Prateek Nayak <kprateek.nayak@....com>,
	"Gautham R . Shenoy" <gautham.shenoy@....com>, Vincent Guittot
	<vincent.guittot@...aro.org>, Juri Lelli <juri.lelli@...hat.com>, "Dietmar
 Eggemann" <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, "Valentin
 Schneider" <vschneid@...hat.com>, Libo Chen <libo.chen@...cle.com>, "Madadi
 Vineeth Reddy" <vineethr@...ux.ibm.com>, Hillf Danton <hdanton@...a.com>,
	Shrikanth Hegde <sshegde@...ux.ibm.com>, Jianyong Wu
	<jianyong.wu@...look.com>, Yangyu Chen <cyy@...self.name>, Tingyin Duan
	<tingyin.duan@...il.com>, Vern Hao <vernhao@...cent.com>, Len Brown
	<len.brown@...el.com>, Tim Chen <tim.c.chen@...ux.intel.com>, Aubrey Li
	<aubrey.li@...el.com>, Zhao Liu <zhao1.liu@...el.com>, Chen Yu
	<yu.chen.surf@...il.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v4 05/28] sched: Add hysteresis to switch a task's
 preferred LLC

On 9/29/2025 9:44 PM, Peter Zijlstra wrote:
> On Sat, Aug 09, 2025 at 01:02:18PM +0800, Chen Yu wrote:
>> From: Tim Chen <tim.c.chen@...ux.intel.com>
>>
>> Switching a process's preferred LLC generates lots of task
>> migrations across LLCs. To avoid frequent switches
>> of home LLC, implement the following policy:
>>
>> 1. Require a 2x occ change threshold to switch preferred LLC
>> 2. Don't discard preferred LLC for a task
>>
>> Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
>> ---
>>   kernel/sched/fair.c | 24 ++++++++++++++++--------
>>   1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index a7be5c5ecba3..9e3c6f0eb934 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -1175,6 +1175,14 @@ static s64 update_curr_se(struct rq *rq, struct sched_entity *curr)
>>   #define EPOCH_PERIOD	(HZ/100)	/* 10 ms */
>>   #define EPOCH_OLD	5		/* 50 ms */
>>   
>> +static int llc_id(int cpu)
>> +{
>> +	if (cpu < 0)
>> +		return -1;
>> +
>> +	return per_cpu(sd_llc_id, cpu);
>> +}
>> +
>>   void mm_init_sched(struct mm_struct *mm, struct mm_sched __percpu *_pcpu_sched)
>>   {
>>   	unsigned long epoch;
>> @@ -1307,6 +1315,7 @@ static void __no_profile task_cache_work(struct callback_head *work)
>>   	struct task_struct *p = current;
>>   	struct mm_struct *mm = p->mm;
>>   	unsigned long m_a_occ = 0;
>> +	unsigned long last_m_a_occ = 0;
>>   	int cpu, m_a_cpu = -1;
>>   	cpumask_var_t cpus;
>>   
>> @@ -1345,11 +1354,13 @@ static void __no_profile task_cache_work(struct callback_head *work)
>>   					     per_cpu(sd_llc_id, i), occ, m_occ, m_cpu, nr);
>>   			}
>>   
>> -			a_occ /= nr;
>> +			// a_occ /= nr;
> 
> This seems broken.
> 

The original thought was that not dividing the nr might help in
an asymmetric LLC scenario.

If there are 2 threads in the 4-CPU LLC and 3 threads in the
8-CPU LLC, it might be better to choose the one with 3 threads.
But it wouldn't be the case if division is done by the nr.

And the NUMA balancing fault statistic behaves similarly:
it compares the total number of faults per node rather than
the average number of faults per CPU, IIUC.

>>   			if (a_occ > m_a_occ) {
>>   				m_a_occ = a_occ;
>>   				m_a_cpu = m_cpu;
>>   			}
>> +			if (llc_id(cpu) == llc_id(mm->mm_sched_cpu))
>> +				last_m_a_occ = a_occ;
> 
> Not 'last', 'curr' perhaps?
> 

OK.

>>   
>>   			trace_printk("(%d) a_occ: %ld m_a_occ: %ld\n",
>>   				     per_cpu(sd_llc_id, cpu), a_occ, m_a_occ);
>> @@ -1363,13 +1374,10 @@ static void __no_profile task_cache_work(struct callback_head *work)
>>   		}
>>   	}
>>   
>> -	/*
>> -	 * If the max average cache occupancy is 'small' we don't care.
>> -	 */
>> -	if (m_a_occ < (NICE_0_LOAD >> EPOCH_OLD))
>> -		m_a_cpu = -1;
>> -
>> -	mm->mm_sched_cpu = m_a_cpu;
>> +	if (m_a_occ > (2 * last_m_a_occ)) {
>> +		/* avoid the bouncing of mm_sched_cpu */
>> +		mm->mm_sched_cpu = m_a_cpu;
>> +	}
> 
> 
> The whole double thing doesn't seem right either. That means that
> anything over .5 will never be able to change, even when confronted with
> say a .8.
> 
> Also, while this is a threshold, this is not in fact hysteresis.
> 

I agree 2X might be too aggressive. The threshold here was intended
to avoid switching to a new mm_sched_cpu too quickly, but it is not
intended to address the back-and-forth bouncing issue that hysteresis
would resolve. Instead, the task migration bouncing issue is solved
in patch 7/28 via a hysteresis.

Maybe the comment above:
/* avoid the bouncing of mm_sched_cpu */
should be changed to:
/* avoid switching to mm_sched_cpu too fast */

I'll investigate an imbalance_pct value that is less aggressive than 2X.


thanks,
Chenyu