From: Frederic Weisbecker <frederic@kernel.org>
To: Radu Toader <radu.m.toader@gmail.com>
Cc: peterz@infradead.org, mingo@redhat.com, tglx@kernel.org,
tglx@linutronix.de, tj@kernel.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, kprateek.nayak@amd.com, void@manifault.com,
arighi@nvidia.com, changwoo@igalia.com, anna-maria@linutronix.de,
bigeasy@linutronix.de, joshdon@google.com, haoluo@google.com,
brho@google.com, boqun@kernel.org, joelagnelf@nvidia.com,
enlin.mu@linux.dev, brads@mainlining.org, kmehltretter@gmail.com,
sched-ext@lists.linux.dev, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] softirq: Treat an interrupted idle task as idle in tick_irq_exit()
Date: Wed, 11 Nov 2026 17:10:54 +0100 [thread overview]
Message-ID: <avSTjihu0YXfFNUV@localhost.localdomain> (raw)
In-Reply-To: <20261111112944.3868822-1-radu.m.toader@gmail.com>
Le Wed, Nov 11, 2026 at 11:29:44AM +0000, Radu Toader a écrit :
> tick_irq_exit() only runs tick_nohz_irq_exit() if sched_core_idle_cpu(),
> which is idle_cpu() unless core scheduling is built in. idle_cpu() is
> false when the run queue has tasks while the idle task runs.
>
> That is the normal state of a CPU under sched_ext whose BPF scheduler
> holds back a runnable task of that CPU in one of its queues, e.g. a task
> of a cgroup at its CPU limit: the task stays queued on the CPU's run
> queue and counts in rq->nr_running, while the CPU sits in the idle loop.
>
> tick_nohz_irq_exit() is what marks an idle CPU as idle again for the
> scheduler clock after an interrupt in the idle loop
> (TS_FLAG_IDLE_ACTIVE, sched_clock_idle_sleep_event()). Without it, the
> next exit from the idle loop doesn't resynchronize the CPU's sched_clock
> with sched_clock_idle_wakeup_event(). With an unstable sched_clock, e.g.
> under kvm-clock without a stable TSC, the clock of a CPU in tick-stopped
> idle is clamped to its last stamp plus a tick, so a task picked after a
> reschedule IPI (which doesn't go through irq_enter()) starts with a
> stale exec_start, and the next tick charges the whole idle stretch to
> it.
>
> Measured with one thread spinning 10 ms in every 100 ms in a cgroup,
> kept off its CPU for a further 7 to 53 ms per period by a sched_ext
> scheduler: the cgroup's cpu.stat usage_usec over the thread's own
> CLOCK_MONOTONIC measurement of its busy time reached 2.00, twice the
> CPU time the thread could possibly have used, and is 0.96 with this
> patch. The same ratio with no BPF scheduler loaded is 1.005.
>
> Commit 548796e2e70b ("sched/core: introduce sched_core_idle_cpu()")
> fixed this same gate for the other case of "the idle task runs while
> nr_running > 0", forced idle under core scheduling, where the symptom
> was too little idle time in /proc/stat. sched_ext made that state
> reachable without CONFIG_SCHED_CORE, and there the symptom is idle time
> charged to a task as runtime.
>
> Ask whether the idle task is running, which covers both cases. CPUs
> whose run queue is really empty are unaffected; a CPU interrupted
> between leaving the idle loop and switching to a task has need_resched()
> set or has already left the idle accounting, see tick_nohz_irq_exit().
> sched_core_idle_cpu() has no users left and goes.
>
> The two gates test different predicates rather than one containing the
> other, and the difference is deliberate. idle_cpu() is idle_rq():
> rq->curr == rq->idle && !rq->nr_running && !rq->ttwu_pending.
> is_idle_task() tests PF_IDLE on current. Newly covered:
>
> - the two "the idle task runs while nr_running > 0" cases above;
>
> - a CPU idling with a wakeup queued by TTWU_QUEUE whose IPI has not
> run yet;
>
> - injected idle. play_idle_precise() sets PF_IDLE on a SCHED_FIFO
> kthread which then loops on do_idle(), so rq->curr is not rq->idle
> there and the old gate was false off nohz_full CPUs. Interrupts
> taken during injected idle now run tick_nohz_irq_exit() with
> TS_FLAG_INIDLE set. That CPU is running an idle state and needs the
> same latch for the same reason, so this is wanted rather than merely
> harmless, but note it reaches the idle injection users in
> drivers/powercap/idle_inject.c, intel_powerclamp and
> cpuidle_cooling, and is not confined to sched_ext.
>
> One state is no longer covered: PF_IDLE is set by cpu_startup_entry()
> and not by init_idle(), so a CPU on its idle task before that point
> satisfied idle_cpu() but does not satisfy is_idle_task(current). Nothing
> is lost, because TS_FLAG_INIDLE is only set inside do_idle(), so
> tick_nohz_irq_exit() had nothing to do there, and nohz_full CPUs are
> covered by tick_nohz_full_cpu(cpu) either way.
>
> What keeps the new gate from giving a CPU idle accounting it should not
> have is that tick_nohz_irq_exit() tests TS_FLAG_INIDLE itself and only
> does the idle bookkeeping inside the idle loop.
>
> Tooling: the bug was found and this patch and changelog were written by
> Claude Opus 5 (Claude Code)
About the changelog, I could recognize given the overzealous details and
labyrinthic reasoning :-)
Still it looks correct.
> in a session measuring cgroup CPU limit
> enforcement under sched_ext. All of the patch and all of the changelog
> are tool-generated. The measurements above were taken on hosts that no
> longer exist and were not repeated for this posting. By the threat model
> in Documentation/process/threat-model.rst this is a regular bug and not
> a vulnerability: it misreports CPU time under a legitimate configuration
> and crosses no privilege or isolation boundary.
>
> Fixes: f0e1a0643a59 ("sched_ext: Implement BPF extensible scheduler class")
> Link: https://lore.kernel.org/lkml/20261106094343.174905-2-radu.m.toader@gmail.com/
> Signed-off-by: Radu Toader <radu.m.toader@gmail.com>
> ---
> Changes in v3:
> - No code change; the diff is identical to v2.
> - Moved the posting notes out of the changelog and below this line, as
> asked in review of v2. The Link: tag still points at the reproducer.
> - The Tooling: line stays in the changelog above the --- line, because
> Documentation/process/generated-content.rst asks for tool use to be
> noted there.
>
> The reproducer is a sched_ext scheduler that parks a tagged task on an
> otherwise idle CPU for a set time. It was patch 1 of v1 and is dropped
> from this posting at the maintainer's request; it remains in the list
> archive, see the Link: tag above.
>
> Compile-tested in four x86_64 defconfig variants - SCHED_CORE=y with
> NO_HZ_FULL=y, SCHED_CORE=n, NO_HZ_COMMON=n, and SCHED_CLASS_EXT=y with
> SCHED_CORE=y and NO_HZ_FULL=y - with no new warnings at W=1.
>
> v1: https://lore.kernel.org/lkml/20261106094343.174905-1-radu.m.toader@gmail.com/
> v2 and its CI run: https://patchwork.kernel.org/series/1206104/
>
> include/linux/sched.h | 2 --
> kernel/sched/syscalls.c | 12 ------------
> kernel/softirq.c | 12 ++++++++++--
> 3 files changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 1c93da75226e..0e704ea9416f 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2326,11 +2326,9 @@ extern void sched_core_free(struct task_struct *tsk);
> extern void sched_core_fork(struct task_struct *p);
> extern int sched_core_share_pid(unsigned int cmd, pid_t pid, enum pid_type type,
> unsigned long uaddr);
> -extern int sched_core_idle_cpu(int cpu);
> #else
> static inline void sched_core_free(struct task_struct *tsk) { }
> static inline void sched_core_fork(struct task_struct *p) { }
> -static inline int sched_core_idle_cpu(int cpu) { return idle_cpu(cpu); }
> #endif
>
> extern void sched_set_stop_task(int cpu, struct task_struct *stop);
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index 45a127deeadd..22e316680a86 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -194,18 +194,6 @@ struct task_struct *idle_task(int cpu)
> return cpu_rq(cpu)->idle;
> }
>
> -#ifdef CONFIG_SCHED_CORE
> -int sched_core_idle_cpu(int cpu)
> -{
> - struct rq *rq = cpu_rq(cpu);
> -
> - if (sched_core_enabled(rq) && rq->curr == rq->idle)
> - return 1;
> -
> - return idle_cpu(cpu);
> -}
> -#endif /* CONFIG_SCHED_CORE */
> -
> /**
> * find_process_by_pid - find a process with a matching PID value.
> * @pid: the pid in question.
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 7fc2200392cd..e7d649c29c0c 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -738,8 +738,16 @@ static inline void tick_irq_exit(void)
> #ifdef CONFIG_NO_HZ_COMMON
> int cpu = smp_processor_id();
>
> - /* Make sure that timer wheel updates are propagated */
> - if ((sched_core_idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu)) {
> + /*
> + * Make sure that timer wheel updates are propagated. An interrupted
> + * idle loop counts as idle even with runnable tasks queued on the
> + * CPU: forced idle under core scheduling, and the tasks sched_ext
> + * keeps on a CPU's run queue while its BPF scheduler holds them back.
> + * idle_cpu() would skip the idle bookkeeping of tick_nohz_irq_exit()
> + * for those, and with an unstable sched_clock the CPU's clock then
> + * isn't resynchronized when it leaves the idle loop.
> + */
> + if ((is_idle_task(current) && !need_resched()) || tick_nohz_full_cpu(cpu)) {
> if (hardirq_count() == HARDIRQ_OFFSET)
> tick_nohz_irq_exit();
Works for me, but Peter might find something I haven't.
Acked-by: Frederic Weisbecker <frederic@kernel.org>
> }
>
> base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
> --
> 2.43.0
>
--
Frederic Weisbecker
SUSE Labs
next prev parent reply other threads:[~2026-11-11 16:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-11 11:29 Radu Toader
2026-11-11 16:10 ` Frederic Weisbecker [this message]
2026-11-11 16:31 Bradley Morgan
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=avSTjihu0YXfFNUV@localhost.localdomain \
--to=frederic@kernel.org \
--cc=anna-maria@linutronix.de \
--cc=arighi@nvidia.com \
--cc=bigeasy@linutronix.de \
--cc=boqun@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brads@mainlining.org \
--cc=brho@google.com \
--cc=bsegall@google.com \
--cc=changwoo@igalia.com \
--cc=dietmar.eggemann@arm.com \
--cc=enlin.mu@linux.dev \
--cc=haoluo@google.com \
--cc=joelagnelf@nvidia.com \
--cc=joshdon@google.com \
--cc=juri.lelli@redhat.com \
--cc=kmehltretter@gmail.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=radu.m.toader@gmail.com \
--cc=rostedt@goodmis.org \
--cc=sched-ext@lists.linux.dev \
--cc=tglx@kernel.org \
--cc=tglx@linutronix.de \
--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
all inboxes | Powered by JetHome®