mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Andrea Righi <arighi@nvidia.com>,
	mingo@kernel.org, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	vschneid@redhat.com, linux-kernel@vger.kernel.org, tj@kernel.org,
	ziqianlu@bytedance.com
Subject: Re: [PATCH v2 12/12] sched: Remove sched_class::balance()
Date: Thu, 12 Nov 2026 11:34:28 +0100	[thread overview]
Message-ID: <20261112103428.GL2668804@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <87ed02f4-f510-47a7-b3fa-30ad7909cb24@amd.com>

On Thu, Nov 12, 2026 at 03:47:00PM +0530, K Prateek Nayak wrote:

> >> In that case, why should RETRY_TASK want to try balance again for the old
> >> "rq->donor" for which we have already ran balanced for? Shouldn't it simply
> >> pick the task then? Am I missing something?
> > 
> > need_pull_dl_task() has '&& dl_task(prev)', so if prev was FAIR, then it
> > will not be true and pick will not cause it to pull another task. And
> > similar for RT, it checks if the current highest prio is less than the
> > previous prio, and if prev is FAIR that won't be the case.
> 
> Ack, but assume:
> 
>   /* RT tasks blocks */
>   pick_task_rt()
>     balance_rt()
>     /* Fails */
> 
>   pick_task_fair()
>     sched_balance_newidle()
>       raw_spin_rq_unlock()
> 
>       /* New RT task wakes up */
>       
>       raw_spin_rq_lock()
>       if (rq_modified_above()) /* True */
>         return -1
>      return RETRY_TASK;
> 
> 
>   pick_task_rt()
>     balance_rt()
>      need_pull_rt_task() / *Still true */
>        /* Again: Probably unnecessary? */
> 
>     /* But it does find an RT task so loop ends */ 

Ah, yes, in this case we appear to have an unnecessary balance.


> > So the problem is with ext :/ They need balance and pick to be one. If
> > we have separate loops there is the possibility of doing a balance but
> > then not picking it. In that case ext needs to undo the balance and we
> > don't have anything for that.
> > 
> > Humm...
> > 
> > Ah! we now have consistent cross-class wakeup_preempt calls. If we have
> > the following scenario:
> > 
> >   - only EXT tasks
> >   - balance_ext(), moves task from global DSQ to local DSQ
> >   - wakeup FAIR
> >       wakeup_preempt
> >         if (sched_class_above(p->sched_class, rq->next_class))
> > 	  rq->next_class->wakeup_preempt(rq, p, flags)
> > 	    // could move task back to global DSQ
> 
> This part I'm slightly confused about (on tip:sched/core):
> 
>   prev was RT
> 
>   pick_task_scx()
>     do_pick_task_scx()
> 
>       rq_modified_begin(rq, &ext_sched_class);
> 
>       dispatch_pick()
>         dispatch_one()
>           ops.acquire() <-- CPU is back in control of ext
> 
>           /* Pulls a task to local DSQ */
> 
>           /* A new RT task wakes up */
> 
>       if (!force_scx && rq_modified_above(rq, &ext_sched_class)) /* True for normal pick */
> 	return RETRY_TASK;
> 
>   ... goes and picks next RT task
> 
> 
> The ops.release() is only called when put_prev_task_scx() sees next of
> higher class which means the local dispatch from the pick stays that
> way until it is picked to run.
> 
> So, the question is, should balance_ext() do anything more than what
> pick is doing today to pull the task if local DSQ is empty? Am I
> missing something?

Aah, wakeup_preempt_scx() already does what it needed here. See how it
calls scx_schedule_reenq_local() when p's sched class isn't ext.


  reply	other threads:[~2026-11-12 10:34 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-07 11:59 [PATCH v2 00/12] sched: core-sched fixes and balancing Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 01/12] sched/core: Fix pick_next_task() self recursion Peter Zijlstra
2026-11-11  0:30   ` Tejun Heo
2026-11-11 10:45     ` Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 02/12] sched/core: Introduce rq_drop_lock() Peter Zijlstra
2026-11-08  9:25   ` Kayra Cizmeci
2026-11-11 10:46     ` Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 03/12] sched/core: Decompose update_rq_clock() Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 04/12] sched/core: Make __update_rq_clock() skip updates on UPDATED Peter Zijlstra
2026-11-11  1:11   ` Tejun Heo
2026-11-11 10:28     ` Peter Zijlstra
2026-11-11 12:25       ` Peter Zijlstra
2026-11-11 17:08         ` Tejun Heo
2026-11-07 11:59 ` [PATCH v2 05/12] sched/core: Abstract/fix rq_clock_start_loop_update() Peter Zijlstra
2026-11-08 22:26   ` Kayra Cizmeci
2026-11-11  9:10     ` K Prateek Nayak
2026-11-11  9:51       ` Kayra Cizmeci
2026-11-11 16:39     ` Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 06/12] sched/core: New rq_{pin,drop,repin,unpin}_lock() semantics Peter Zijlstra
2026-11-11 10:41   ` Tejun Heo
2026-11-07 11:59 ` [PATCH v2 07/12] sched/core: Simplify/fix time updates Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 08/12] sched/core: Allow newidle for core-sched Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 09/12] sched/rt: Add early exit on balance path Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 10/12] sched/fair: Reflow pick_task_fair() / newidle Peter Zijlstra
2026-11-11 11:02   ` Tejun Heo
2026-11-11 12:31     ` Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 11/12] sched/fair: Push sched_balance_newidle() unlock down Peter Zijlstra
2026-11-07 11:59 ` [PATCH v2 12/12] sched: Remove sched_class::balance() Peter Zijlstra
2026-11-11 11:18   ` Tejun Heo
2026-11-11 13:05     ` Peter Zijlstra
2026-11-11 13:11       ` Peter Zijlstra
2026-11-11 14:57         ` Kayra Cizmeci
2026-11-11 15:47           ` Peter Zijlstra
2026-11-11 16:15             ` Peter Zijlstra
2026-11-11 18:53               ` Kayra Cizmeci
2026-11-12  6:46   ` K Prateek Nayak
2026-11-12  9:33     ` Peter Zijlstra
2026-11-12 10:17       ` K Prateek Nayak
2026-11-12 10:34         ` Peter Zijlstra [this message]
2026-11-11 11:46 ` [PATCH v2 00/12] sched: core-sched fixes and balancing Tejun Heo
2026-11-11 13:57   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261112103428.GL2668804@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arighi@nvidia.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=ziqianlu@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®