From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Aaron Tomlin <atomlin@atomlin.com>, <mingo@redhat.com>,
<peterz@infradead.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>
Cc: <sshegde@linux.ibm.com>, <neelx@suse.com>, <sean@ashe.io>,
<mproche@gmail.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] sched/deadline: Log Fair Server re-enablement for symmetry with debugfs
Date: Fri, 9 Jan 2026 11:47:10 +0530 [thread overview]
Message-ID: <0e76c836-c645-4fd0-9d86-b47b8834eac2@amd.com> (raw)
In-Reply-To: <20260109031959.2786873-2-atomlin@atomlin.com>
On 1/9/2026 8:49 AM, Aaron Tomlin wrote:
> Currently, the scheduler's debug interface emits a notification to the
> console when the Fair Server is explicitly disabled via the fair_server
> sysfs attribute. However, no corresponding log entry is generated when
> the server is subsequently re-enabled.
>
> This omission results in an asymmetry within the kernel logs,
> potentially obscuring the true operational state of the scheduler during
> debugging or performance analysis.
Well, if you are disabling the fair_server, you're opening the doors to
bigger problems and that printk mainly serves as an indicator to dismiss
user induced starvation issues during debugs.
Why do you care about the symmetry of this log when you shouldn't be
setting the runtime to 0 in the first place?
>
> This patch amends dl_server_apply_params() to introduce the requisite
> logging. By detecting the transition from zero to non-zero
> bandwidth - strictly for the Fair Server entity and excluding
> initialisation - we ensure that a "Fair server re-enabled" message is
> emitted. This restores logging symmetry and provides administrators with
> a clear audit trail of manual runtime adjustments.
>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
> kernel/sched/deadline.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 319439fe1870..e64fb988e957 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1867,6 +1867,7 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio
> u64 old_bw = init ? 0 : to_ratio(dl_se->dl_period, dl_se->dl_runtime);
> u64 new_bw = to_ratio(period, runtime);
> struct rq *rq = dl_se->rq;
> + bool fair_server = dl_se == &rq->fair_server;
> int cpu = cpu_of(rq);
> struct dl_bw *dl_b;
> unsigned long cap;
> @@ -1876,6 +1877,11 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio
> dl_b = dl_bw_of(cpu);
> guard(raw_spinlock)(&dl_b->lock);
>
> + /* Symmetric to disable message in sched_fair_server_write() */
> + if (!init && fair_server && !old_bw && new_bw)
> + printk_deferred("Fair server re-enabled on CPU %d.\n",
> + cpu);
That is an absolutely terrible place to put it. Why can't we have it in
sched_fair_server_write() for DL_RUNTIME when the
"rq->fair_server.dl_runtime" is 0 initially and is modified to a
non-zero value similar to the "Fair server disabled" message?
I still think once the fair server is disabled, the pieces are for the
user to keep. I wouldn't want us debugging:
Fair server disabled in CPU X ...
Fair server re-enabled in CPU X ...
INFO: rcu_tasks detected stalls ...
only to realise the stalls were a result of starving the fair threads
and the fair server didn't run in time / didn't have enough B/W to
prevent that stall.
> +
> cpus = dl_bw_cpus(cpu);
> cap = dl_bw_capacity(cpu);
>
--
Thanks and Regards,
Prateek
next prev parent reply other threads:[~2026-01-09 6:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-09 3:19 [PATCH 0/1] sched/deadline: Log Fair Server re-enablement Aaron Tomlin
2026-01-09 3:19 ` [PATCH 1/1] sched/deadline: Log Fair Server re-enablement for symmetry with debugfs Aaron Tomlin
2026-01-09 6:17 ` K Prateek Nayak [this message]
2026-01-09 14:30 ` Aaron Tomlin
2026-01-12 5:14 ` K Prateek Nayak
2026-01-12 14:32 ` Aaron Tomlin
2026-01-12 18:48 ` Shrikanth Hegde
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=0e76c836-c645-4fd0-9d86-b47b8834eac2@amd.com \
--to=kprateek.nayak@amd.com \
--cc=atomlin@atomlin.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=mproche@gmail.com \
--cc=neelx@suse.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=sean@ashe.io \
--cc=sshegde@linux.ibm.com \
--cc=vincent.guittot@linaro.org \
--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
all inboxes | Powered by JetHome®