mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: 'Tejun Heo' <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: liuwenfang <liuwenfang@honor.com>,
	'David Vernet' <void@manifault.com>,
	'Andrea Righi' <arighi@nvidia.com>,
	'Changwoo Min' <changwoo@igalia.com>,
	'Ingo Molnar' <mingo@redhat.com>,
	'Juri Lelli' <juri.lelli@redhat.com>,
	'Vincent Guittot' <vincent.guittot@linaro.org>,
	'Dietmar Eggemann' <dietmar.eggemann@arm.com>,
	'Steven Rostedt' <rostedt@goodmis.org>,
	'Ben Segall' <bsegall@google.com>, 'Mel Gorman' <mgorman@suse.de>,
	'Valentin Schneider' <vschneid@redhat.com>,
	"'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>,
	Joel Fernandes <joelagnelf@nvidia.com>
Subject: Re: [PATCH v4 2/3] sched_ext: Fix cpu_released while RT task and SCX task are scheduled concurrently
Date: Tue, 19 Aug 2025 14:28:17 -1000	[thread overview]
Message-ID: <aKUWoePcNPcnJT1D@slm.duckdns.org> (raw)
In-Reply-To: <20250819074736.GD3245006@noisy.programming.kicks-ass.net>

Hello, Peter.

(cc'ing Joel for the @rf addition to pick_task())

On Tue, Aug 19, 2025 at 09:47:36AM +0200, Peter Zijlstra wrote:
...
> You're now asking for a 3rd call out to do something like:
> 
>   ->balance() -- ready a task for pick
>   ->pick() -- picks a random other task
>   ->put_prev() -- oops, our task didn't get picked, stick it back
> 
> Which is bloody ludicrous. So no. We're not doing this.
> 
> Why can't pick DTRT ?

This is unfortunate, but, given how things are set up right now, I think we
probably need the last one. Taking a step back and also considering the
proposed @rf addition to pick():

- The reason why SCX needs to do most of its dispatch operations in
  balance() is because the kernel side doesn't know which tasks are going to
  execute on which CPU until the task is actually picked for execution, so
  all picking must be preceded by balance() where tasks can be moved across
  rqs.

- There's a gap between balance() and pick_task() where a successful return
  from balance() doesn't guarantee that the corresponding pick() would be
  called. This seems intentional to guarantee that no matter what happens
  during balance(), pick_task() of the highest priority class with a pending
  task is guaranteed to get the CPU.

  This guarantee changes if we add @rf to pick_task() and let it unlock and
  relock. A higher priority task may get queued while the rq lock is
  released and then the lower priority pick_task() may still return a task
  of its own. This should be resolvable although it may not be completely
  trivial. We need to shift clear_tsk_need_resched() before pick_task()'s
  and then make wakeup_preempt() would probalby need more complications to
  guarantee that resched_curr() is not skipped while scheduling is taking
  place.

- SCX's ops.cpu_acquire() and .cpu_release() are to tell the BPF scheduler
  that a CPU is available for running SCX tasks or not. We want to tell the
  BPF side that a CPU became available before its ops.dispatch() is called -
  ie. before balance(). So, IIUC, this is where the problem is. Because
  there's a gap between balance() and pick_task(), the CPU might get taken
  by a higher priority sched class inbetween. If that happens, we need to
  tell the BPF scheduler that it lost the CPU. However, if the previous task
  wasn't a SCX one, there's currently no place to tell so.

  IOW, SCX needs to invoke ops.cpu_released() when a CPU is taken between
  its balance() and pick_task(); however, that can happen when both prev and
  next tasks are !SCX tasks, so it needs something which is always called.

If @rf is added to pick_task() so that we can merge balance() into
pick_task(), that'd be simplify these. SCX wouldn't need balance index
boosting and can handle cpu_acquire/release() within pick_task(). What do
you think?

Thanks.

-- 
tejun

  parent reply	other threads:[~2025-08-20  0:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-21  4:09 [PATCH] " liuwenfang
2025-06-23 19:50 ` 'Tejun Heo'
2025-06-28  6:50   ` [PATCH v2 1/2] " liuwenfang
2025-07-17 21:38     ` 'Tejun Heo'
2025-07-20  9:20       ` liuwenfang
2025-07-20  9:38         ` [PATCH v3 2/3] " liuwenfang
2025-08-12  1:26           ` 'Tejun Heo'
2025-07-20  9:41         ` [PATCH v3 3/3] sched_ext: Fix cpu_released while changing sched policy of the running task liuwenfang
2025-08-12  1:31           ` 'Tejun Heo'
2025-08-19  6:52           ` [PATCH v4 1/3] sched_ext: Fix pnt_seq calculation when picking the next task liuwenfang
2025-08-19  6:55             ` [PATCH v4 2/3] sched_ext: Fix cpu_released while RT task and SCX task are scheduled concurrently liuwenfang
2025-08-19  7:07               ` [PATCH v4 3/3] sched_ext: Fix cpu_released while changing sched policy of the running task liuwenfang
2025-08-19  7:47               ` [PATCH v4 2/3] sched_ext: Fix cpu_released while RT task and SCX task are scheduled concurrently Peter Zijlstra
2025-08-19  8:47                 ` 回复: " liuwenfang
2025-08-19 10:08                   ` Peter Zijlstra
2025-08-20  0:28                 ` 'Tejun Heo' [this message]
2025-08-20  9:18                   ` Peter Zijlstra
2025-08-20 16:52                     ` 'Tejun Heo'
2025-06-28  7:20   ` [PATCH v2 2/2] sched_ext: Fix cpu_released while changing sched policy of the running task liuwenfang
2025-07-17 21:48     ` 'Tejun Heo'
2025-07-18  9:06       ` liuwenfang
2025-07-20  9:36         ` [PATCH v3 1/3] sched_ext: Fix pnt_seq calculation liuwenfang
2025-08-12  0:03           ` 'Tejun Heo'
2025-08-12  0:30             ` 'Tejun Heo'
2025-08-18 10:45               ` liuwenfang
2025-08-18 17:43                 ` 'Tejun Heo'
2025-08-19  7:41                   ` liuwenfang
2025-08-18 17:47           ` Peter Zijlstra
2025-08-19  7:36             ` liuwenfang

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=aKUWoePcNPcnJT1D@slm.duckdns.org \
    --to=tj@kernel.org \
    --cc=arighi@nvidia.com \
    --cc=bsegall@google.com \
    --cc=changwoo@igalia.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joelagnelf@nvidia.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuwenfang@honor.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=void@manifault.com \
    --cc=vschneid@redhat.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

Powered by JetHome