linux-kernel - Re: [PATCH v2 1/3] sched/fair: Fix warning if 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: <20241129095500.GD15382@noisy.programming.kicks-ass.net>
Date: Fri, 29 Nov 2024 10:55:00 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: Adam Li <adamli@...amperecomputing.com>, 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 v2 1/3] sched/fair: Fix warning if NEXT_BUDDY enabled

On Thu, Nov 28, 2024 at 12:59:54PM +0530, K Prateek Nayak wrote:

> I tried to put a SCHED_WARN_ON() here to track where this comes from and
> seems like it is usually from attach_task() in the load balancing path
> pulling a delayed task which is set as the next buddy in
> check_preempt_wakeup_fair()
> 
> Can you please try the following diff instead of the first two patches
> and see if you still hit these warnings, stalls, and pick_eevdf()
> returning NULL?
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ff7cae9274c5..61e74eb5af22 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5478,6 +5478,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  	bool sleep = flags & DEQUEUE_SLEEP;
>  	update_curr(cfs_rq);
> +	clear_buddies(cfs_rq, se);
>  	if (flags & DEQUEUE_DELAYED) {
>  		SCHED_WARN_ON(!se->sched_delayed);
> @@ -5520,8 +5521,6 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  	update_stats_dequeue_fair(cfs_rq, se, flags);
> -	clear_buddies(cfs_rq, se);
> -
>  	update_entity_lag(cfs_rq, se);
>  	if (sched_feat(PLACE_REL_DEADLINE) && !sleep) {
>  		se->deadline -= se->vruntime;

So this puts the clear_buddies() before the whole delayed thing, and
should be sufficient afaict, no?

> @@ -8767,7 +8766,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>  	if (unlikely(throttled_hierarchy(cfs_rq_of(pse))))
>  		return;
> -	if (sched_feat(NEXT_BUDDY) && !(wake_flags & WF_FORK)) {
> +	if (sched_feat(NEXT_BUDDY) && !(wake_flags & WF_FORK) && !pse->sched_delayed) {
>  		set_next_buddy(pse);
>  	}

But then this should never happen, which is after a wakeup, p and the
whole hierarchy up should be runnable at this point.

Or should I go find more wake-up juice and try again :-)



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 :/