From: Andrea Righi <arighi@nvidia.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: 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>,
Tejun Heo <tj@kernel.org>, Joel Fernandes <joelagnelf@nvidia.com>,
David Vernet <void@manifault.com>,
Changwoo Min <changwoo@igalia.com>,
Daniel Hodges <hodgesd@meta.com>,
Christian Loehle <christian.loehle@arm.com>,
Emil Tsalapatis <emil@etsalapatis.com>,
sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] sched/debug: Stop and start server based on if it was active
Date: Tue, 3 Feb 2026 14:50:06 +0100 [thread overview]
Message-ID: <aYH9Ds0yy6cRxuZI@gpd4> (raw)
In-Reply-To: <20260203103407.GK1282955@noisy.programming.kicks-ass.net>
On Tue, Feb 03, 2026 at 11:34:07AM +0100, Peter Zijlstra wrote:
> On Mon, Feb 02, 2026 at 11:37:31PM +0100, Andrea Righi wrote:
>
> > Or:
> >
> > pr_info("%s server %sabled in CPU %d%s\n",
> > server == &rq->fair_server ? "Fair" : "Ext",
> > runtime ? "en" : "dis",
> > cpu_of(rq),
> > runtime ? "" : ", system may crash due to starvation");
>
> Yeah, I noticed it was a bit wonkey. I made it thus.
>
> > > + }
> > > +
> > > *ppos += cnt;
> > > return cnt;
> > > }
> >
> > I like that, it should fix the issue.
>
> There is one more issue when dl_server_apply_params() fails, in that
> case we should test old_runtime to determine if we should (re)start the
> dl_server.
>
> I've ended up with this.
LGTM, I also re-ran all my stress tests with this applied, everything is
working great and the runtime=0 issue is fixed.
Tested-by: Andrea Righi <arighi@nvidia.com>
Thanks!
-Andrea
> Subject: sched/debug: Fix dl_server (re)start conditions
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue Feb 3 11:05:12 CET 2026
>
> There are two problems with sched_server_write_common() that can cause the
> dl_server to malfunction upon attempting to change the parameters:
>
> 1) when, after having disabled the dl_server by setting runtime=0, it is
> enabled again while tasks are already enqueued. In this case is_active would
> still be 0 and dl_server_start() would not be called.
>
> 2) when dl_server_apply_params() would fail, runtime is not applied and does
> not reflect the new state.
>
> Instead have dl_server_start() check its actual dl_runtime, and have
> sched_server_write_common() unconditionally (re)start the dl_server. It will
> automatically stop if there isn't anything to do, so spurious activation is
> harmless -- while failing to start it is a problem.
>
> While there, move the printk out of the locked region and make it symmetric,
> also printing on enable.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> kernel/sched/deadline.c | 5 ++---
> kernel/sched/debug.c | 32 ++++++++++++++------------------
> 2 files changed, 16 insertions(+), 21 deletions(-)
>
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1784,7 +1784,7 @@ void dl_server_start(struct sched_dl_ent
> {
> struct rq *rq = dl_se->rq;
>
> - if (!dl_server(dl_se) || dl_se->dl_server_active)
> + if (!dl_server(dl_se) || dl_se->dl_server_active || !dl_se->dl_runtime)
> return;
>
> /*
> @@ -1882,7 +1882,6 @@ int dl_server_apply_params(struct sched_
> int cpu = cpu_of(rq);
> struct dl_bw *dl_b;
> unsigned long cap;
> - int retval = 0;
> int cpus;
>
> dl_b = dl_bw_of(cpu);
> @@ -1914,7 +1913,7 @@ int dl_server_apply_params(struct sched_
> dl_se->dl_bw = to_ratio(dl_se->dl_period, dl_se->dl_runtime);
> dl_se->dl_density = to_ratio(dl_se->dl_deadline, dl_se->dl_runtime);
>
> - return retval;
> + return 0;
> }
>
> /*
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -338,9 +338,9 @@ static ssize_t sched_server_write_common
> void *server)
> {
> long cpu = (long) ((struct seq_file *) filp->private_data)->private;
> - struct rq *rq = cpu_rq(cpu);
> struct sched_dl_entity *dl_se = (struct sched_dl_entity *)server;
> - u64 runtime, period;
> + u64 old_runtime, runtime, period;
> + struct rq *rq = cpu_rq(cpu);
> int retval = 0;
> size_t err;
> u64 value;
> @@ -350,9 +350,7 @@ static ssize_t sched_server_write_common
> return err;
>
> scoped_guard (rq_lock_irqsave, rq) {
> - bool is_active;
> -
> - runtime = dl_se->dl_runtime;
> + old_runtime = runtime = dl_se->dl_runtime;
> period = dl_se->dl_period;
>
> switch (param) {
> @@ -374,25 +372,23 @@ static ssize_t sched_server_write_common
> return -EINVAL;
> }
>
> - is_active = dl_server_active(dl_se);
> - if (is_active) {
> - update_rq_clock(rq);
> - dl_server_stop(dl_se);
> - }
> -
> + update_rq_clock(rq);
> + dl_server_stop(dl_se);
> retval = dl_server_apply_params(dl_se, runtime, period, 0);
> -
> - if (!runtime)
> - printk_deferred("%s server disabled in CPU %d, system may crash due to starvation.\n",
> - server == &rq->fair_server ? "Fair" : "Ext", cpu_of(rq));
> -
> - if (is_active && runtime)
> - dl_server_start(dl_se);
> + dl_server_start(dl_se);
>
> if (retval < 0)
> return retval;
> }
>
> + if (!!old_runtime ^ !!runtime) {
> + pr_info("%s server %sabled on CPU %d%s.\n",
> + server == &rq->fair_server ? "Fair" : "Ext",
> + runtime ? "en" : "dis",
> + cpu_of(rq),
> + runtime ? "" : ", system may malfunction due to starvation");
> + }
> +
> *ppos += cnt;
> return cnt;
> }
next prev parent reply other threads:[~2026-02-03 13:50 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-26 9:58 [PATCHSET v12 sched_ext/for-6.20] Add a deadline server for sched_ext tasks Andrea Righi
2026-01-26 9:58 ` [PATCH 1/7] sched/deadline: Clear the defer params Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Joel Fernandes
2026-01-26 9:59 ` [PATCH 2/7] sched/debug: Fix updating of ppos on server write ops Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Joel Fernandes
2026-01-26 9:59 ` [PATCH 3/7] sched/debug: Stop and start server based on if it was active Andrea Righi
2026-02-02 21:13 ` Peter Zijlstra
2026-02-02 21:14 ` Peter Zijlstra
2026-02-02 21:17 ` Peter Zijlstra
2026-02-02 22:37 ` Andrea Righi
2026-02-03 10:34 ` Peter Zijlstra
2026-02-03 11:18 ` [tip: sched/core] sched/debug: Fix dl_server (re)start conditions tip-bot2 for Peter Zijlstra
2026-02-03 13:50 ` Andrea Righi [this message]
2026-02-03 10:11 ` [PATCH 3/7] sched/debug: Stop and start server based on if it was active Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Joel Fernandes
2026-01-26 9:59 ` [PATCH 4/7] sched_ext: Add a DL server for sched_ext tasks Andrea Righi
2026-02-02 19:50 ` Peter Zijlstra
2026-02-02 20:32 ` Andrea Righi
2026-02-02 21:10 ` Peter Zijlstra
2026-02-02 22:18 ` Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Andrea Righi
2026-01-26 9:59 ` [PATCH 5/7] sched/debug: Add support to change sched_ext server params Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Joel Fernandes
2026-01-26 9:59 ` [PATCH 6/7] selftests/sched_ext: Add test for sched_ext dl_server Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Andrea Righi
2026-01-26 9:59 ` [PATCH 7/7] selftests/sched_ext: Add test for DL server total_bw consistency Andrea Righi
2026-02-03 11:18 ` [tip: sched/core] " tip-bot2 for Joel Fernandes
2026-02-02 16:45 ` [PATCHSET v12 sched_ext/for-6.20] Add a deadline server for sched_ext tasks Tejun Heo
2026-02-02 19:56 ` Peter Zijlstra
2026-02-02 20:20 ` Tejun Heo
-- strict thread matches above, loose matches on Subject: below --
2026-01-20 21:50 [PATCHSET RESEND v11 " Andrea Righi
2026-01-20 21:50 ` [PATCH 3/7] sched/debug: Stop and start server based on if it was active Andrea Righi
2025-12-17 9:35 [PATCHSET v11 sched_ext/for-6.20] Add a deadline server for sched_ext tasks Andrea Righi
2025-12-17 9:35 ` [PATCH 3/7] sched/debug: Stop and start server based on if it was active Andrea Righi
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=aYH9Ds0yy6cRxuZI@gpd4 \
--to=arighi@nvidia.com \
--cc=bsegall@google.com \
--cc=changwoo@igalia.com \
--cc=christian.loehle@arm.com \
--cc=dietmar.eggemann@arm.com \
--cc=emil@etsalapatis.com \
--cc=hodgesd@meta.com \
--cc=joelagnelf@nvidia.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=sched-ext@lists.linux.dev \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.