linux-kernel - Re: [PATCH v5 08/14] sched/ext: Add a DL server for sched

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: <aFnQ-3L6ixL7nSz0@slm.duckdns.org>
Date: Mon, 23 Jun 2025 12:11:07 -1000
From: Tejun Heo <tj@...nel.org>
To: Joel Fernandes <joelagnelf@...dia.com>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
	Valentin Schneider <vschneid@...hat.com>,
	David Vernet <void@...ifault.com>, Andrea Righi <arighi@...dia.com>,
	Changwoo Min <changwoo@...lia.com>,
	Luigi De Matteis <ldematteis123@...il.com>,
	sched-ext@...ts.linux.dev
Subject: Re: [PATCH v5 08/14] sched/ext: Add a DL server for sched_ext tasks

On Fri, Jun 20, 2025 at 04:32:23PM -0400, Joel Fernandes wrote:
> +static struct task_struct *ext_server_pick_task(struct sched_dl_entity *dl_se,
> +						void *flags)
> +{
> +	struct rq_flags *rf = flags;
> +
> +	balance_scx(dl_se->rq, dl_se->rq->curr, rf);
> +	return pick_task_scx(dl_se->rq, rf);
> +}
...
> +void ext_server_init(struct rq *rq)
> +{
> +	struct sched_dl_entity *dl_se = &rq->ext_server;
> +
> +	init_dl_entity(dl_se);
> +
> +	dl_server_init(dl_se, rq, ext_server_has_tasks, ext_server_pick_task);
> +}

Needing to pass in @rf to ext_server_pick_task() makes sense as SCX always
needs to balance first. However, I still don't understand why that
necessitates adding @rf to sched_class->pick_task(). The existing assumption
is that ->pick_task() cannot release and regrab the rq lock and that's why
->balance() exists in the first place. Breaking this can lead to other
misbehaviors - e.g. if a higher priority class ->pick_task() releases the rq
lock, a lower one may end up losing a task that its ->balance() saw. This
can lead to lower priority class's ->pick_task() being called without its
->balance() being called which can lead to stalls.

One thing that confuses me is that all that the patchset needs to do seems
to be adding @rf to dl_server_pick_f and that seems fine to me. Why is it
necessary to add @rf to sched_class->pick_task()?

Thanks.

-- 
tejun