linux-kernel - Re: [PATCH] sched/fair: Untangle NEXT_BUDDY and pick_next

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: <8e8bdc77-d834-4f9f-ba8e-fe055a476cfe@os.amperecomputing.com>
Date: Fri, 6 Dec 2024 14:47:36 +0800
From: Adam Li <adamli@...amperecomputing.com>
To: Peter Zijlstra <peterz@...radead.org>,
 K Prateek Nayak <kprateek.nayak@....com>
Cc: mingo@...hat.com, juri.lelli@...hat.com, vincent.guittot@...aro.org,
 dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
 mgorman@...e.de, vschneid@...hat.com, linux-kernel@...r.kernel.org,
 patches@...erecomputing.com, cl@...ux.com, christian.loehle@....com,
 vineethr@...ux.ibm.com
Subject: Re: [PATCH] sched/fair: Untangle NEXT_BUDDY and pick_next_task()

On 11/29/2024 6:15 PM, Peter Zijlstra wrote:
> On Fri, Nov 29, 2024 at 10:55:00AM +0100, Peter Zijlstra wrote:
> 
>> Anyway..  I'm sure I started a patch series cleaning up the whole next
>> buddy thing months ago (there's more problems here), but I can't seem to
>> find it in a hurry :/
> 
> There was this..
> 
Hi Peter and Prateek,

I tested the two patches on 6.13-rc1 + patch "sched/fair: Fix NEXT_BUDDY"
(https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=sched/urgent&id=d1139307fe97ffefcf90212772f7516732a11034)
 
1) sched/fair: Untangle NEXT_BUDDY and pick_next_task()
2) sched/fair: Add CGROUP_BUDDY feature

With all 2^3=8 combinations: (NO_)NEXT_BUDDY, (NO_)CGROUP_BUDDY, (NO_)PICK_BUDDY,
there is no warning or panic. There is no significant performance difference for
Specjbb workload.

And there is no much performance difference before and after the two patches.

Before the patches, I think the default setting 'NO_NEXT_BUDDY' logically
equals to 'NO_PICK_BUDDY && CGROUP_BUDDY && NO_NEXT_BUDDY'.
After the patches, the default becomes 'PICK_BUDDY && CGROUP_BUDDY && NO_NEXT_BUDDY'.

Thanks,
-adam
> ---
> Subject: sched/fair: Untangle NEXT_BUDDY and pick_next_task()
> From: Peter Zijlstra <peterz@...radead.org>
> Date: Fri Nov 29 10:36:59 CET 2024
> 
> There are 3 sites using set_next_buddy() and only one is conditional
> on NEXT_BUDDY, the other two sites are unconditional; to note:
> 
>   - yield_to_task()
>   - cgroup dequeue / pick optimization
> 
> However, having NEXT_BUDDY control both the wakeup-preemption and the
> picking side of things means its near useless.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  kernel/sched/fair.c     |    4 ++--
>  kernel/sched/features.h |    9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5613,9 +5613,9 @@ static struct sched_entity *
>  pick_next_entity(struct rq *rq, struct cfs_rq *cfs_rq)
>  {
>  	/*
> -	 * Enabling NEXT_BUDDY will affect latency but not fairness.
> +	 * Picking the ->next buddy will affect latency but not fairness.
>  	 */
> -	if (sched_feat(NEXT_BUDDY) &&
> +	if (sched_feat(PICK_BUDDY) &&
>  	    cfs_rq->next && entity_eligible(cfs_rq, cfs_rq->next)) {
>  		/* ->next will never be delayed */
>  		SCHED_WARN_ON(cfs_rq->next->sched_delayed);
> --- a/kernel/sched/features.h
> +++ b/kernel/sched/features.h
> @@ -32,6 +32,15 @@ SCHED_FEAT(PREEMPT_SHORT, true)
>  SCHED_FEAT(NEXT_BUDDY, false)
>  
>  /*
> + * Allow completely ignoring cfs_rq->next; which can be set from various
> + * places:
> + *   - NEXT_BUDDY (wakeup preemption)
> + *   - yield_to_task()
> + *   - cgroup dequeue / pick
> + */
> +SCHED_FEAT(PICK_BUDDY, true)
> +
> +/*
>   * Consider buddies to be cache hot, decreases the likeliness of a
>   * cache buddy being migrated away, increases cache locality.
>   */
>>
>>