mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: liuwenfang <liuwenfang@honor.com>
Cc: 'Tejun Heo' <tj@kernel.org>, '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>
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 09:47:36 +0200	[thread overview]
Message-ID: <20250819074736.GD3245006@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <8d64c74118c6440f81bcf5a4ac6b9f00@honor.com>


Could you please not thread your new patches onto the old thread? That
makes them near impossible to find.

On Tue, Aug 19, 2025 at 06:55:38AM +0000, liuwenfang wrote:
> Supposed RT task(RT1) is running on CPU0 and RT task(RT2) is awakened on CPU1,
> RT1 becomes sleep and SCX task(SCX1) will be dispatched to CPU0, RT2 will be
> placed on CPU0:
> 
> CPU0(schedule)                                     CPU1(try_to_wake_up)
> set_current_state(TASK_INTERRUPTIBLE)              try_to_wake_up # RT2
> __schedule                                           select_task_rq # CPU0 is selected
> LOCK rq(0)->lock # lock CPU0 rq                        ttwu_queue
>   deactivate_task # RT1                                  LOCK rq(0)->lock # busy waiting
>     pick_next_task # no more RT tasks on rq                 |
>       prev_balance                                          |
>         balance_scx                                         |
>           balance_one                                       |
>             rq->scx.cpu_released = false;                   |
>               consume_global_dsq                            |
>                 consume_dispatch_q                          |
>                   consume_remote_task                       |
>                     UNLOCK rq(0)->lock                      V
>                                                          LOCK rq(0)->lock # succ
>                     deactivate_task # SCX1               ttwu_do_activate
>                     LOCK rq(0)->lock # busy waiting      activate_task # RT2 equeued
>                        |                                 UNLOCK rq(0)->lock
>                        V
>                     LOCK rq(0)->lock # succ
>                     activate_task # SCX1
>       pick_task # RT2 is picked
>       put_prev_set_next_task # prev is RT1, next is RT2, rq->scx.cpu_released = false;
> UNLOCK rq(0)->lock
> 
> At last, RT2 will be running on CPU0 with rq->scx.cpu_released being false, which would
> lead the BPF scheduler to make decisions improperly.
> 
> So, check the sched class in __put_prev_set_next_scx() to fix the value of
> rq->scx.cpu_released.

Oh gawd, this is terrible.

Why would you start the pick in balance and then cry when you fail the
pick in pick ?!?

This is also the reason you need that weird CLASS_EXT exception in
prev_balance(), isn't it?

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 ?

  parent reply	other threads:[~2025-08-19  7:47 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               ` Peter Zijlstra [this message]
2025-08-19  8:47                 ` 回复: [PATCH v4 2/3] sched_ext: Fix cpu_released while RT task and SCX task are scheduled concurrently liuwenfang
2025-08-19 10:08                   ` Peter Zijlstra
2025-08-20  0:28                 ` 'Tejun Heo'
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=20250819074736.GD3245006@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arighi@nvidia.com \
    --cc=bsegall@google.com \
    --cc=changwoo@igalia.com \
    --cc=dietmar.eggemann@arm.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=rostedt@goodmis.org \
    --cc=tj@kernel.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