* Re: [PATCH] sched: Set need-resched flags before tracing
2026-09-11 21:33 [PATCH] sched: Set need-resched flags before tracing Andrea Righi
@ 2026-09-12 7:13 ` Gabriele Monaco
2026-09-12 7:34 ` Andrea Righi
2026-09-12 21:30 ` bot+bpf-ci
1 sibling, 1 reply; 4+ messages in thread
From: Gabriele Monaco @ 2026-09-12 7:13 UTC (permalink / raw)
To: Andrea Righi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann
Cc: Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, bpf, linux-kernel
Il 11 settembre 2026 21:33:00 UTC, Andrea Righi <arighi@nvidia.com> ha scritto:
>sched_set_need_resched_tp is emitted before the corresponding thread
>flag is set. This allows a BPF tracepoint program to recursively invoke
>the same tracepoint while leaving its RCU read-side critical section.
>
>If rcu_read_unlock_special() must defer a quiescent state while
>preemption or interrupts are disabled, it calls
>set_need_resched_current(). Since TIF_NEED_RESCHED is still clear, this
>emits the tracepoint again and repeats until the kernel stack overflows:
>
> __trace_set_need_resched()
> bpf_trace_run3()
> rcu_read_unlock_migrate()
> rcu_read_unlock_special()
> set_need_resched_current()
> set_tsk_need_resched()
> __trace_set_need_resched()
>
>Set the thread flag before emitting the tracepoint in both
>set_tsk_need_resched() and __resched_curr(). For a remote reschedule,
>retain the result of set_nr_and_not_polling() so that tracing remains
>ahead of IPI delivery while still observing the updated flag.
>
>Fixes: adcc3bfa8806 ("sched: Adapt sched tracepoints for RV task model")
>Signed-off-by: Andrea Righi <arighi@nvidia.com>
>---
Hi Andrea,
Isn't this mostly what was done in [1]? I wonder if that patch was just forgotten.
There was a discussion that apparently didn't go anywhere, but it doesn't look like a blocker for the change to me.
Thanks,
Gabriele
[1] - https://lore.kernel.org/lkml/20260627081657.499781-1-rhkrqnwk98@gmail.com
> include/linux/sched.h | 5 ++++-
> kernel/sched/core.c | 7 +++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/include/linux/sched.h b/include/linux/sched.h
>index 59f6366fdf501..6003dcd080e6c 100644
>--- a/include/linux/sched.h
>+++ b/include/linux/sched.h
>@@ -2105,8 +2105,11 @@ static inline int test_tsk_thread_flag(struct task_struct *tsk, int flag)
> static inline void set_tsk_need_resched(struct task_struct *tsk)
> {
> if (tracepoint_enabled(sched_set_need_resched_tp) &&
>- !test_tsk_thread_flag(tsk, TIF_NEED_RESCHED))
>+ !test_tsk_thread_flag(tsk, TIF_NEED_RESCHED)) {
>+ set_tsk_thread_flag(tsk, TIF_NEED_RESCHED);
> __trace_set_need_resched(tsk, TIF_NEED_RESCHED);
>+ return;
>+ }
> set_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> }
>
>diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>index 91f059a556950..aed403fd2f82e 100644
>--- a/kernel/sched/core.c
>+++ b/kernel/sched/core.c
>@@ -1196,6 +1196,7 @@ static void __resched_curr(struct rq *rq, int tif)
> {
> struct task_struct *curr = rq->curr;
> struct thread_info *cti = task_thread_info(curr);
>+ bool send_ipi;
> int cpu;
>
> lockdep_assert_rq_held(rq);
>@@ -1212,15 +1213,17 @@ static void __resched_curr(struct rq *rq, int tif)
>
> cpu = cpu_of(rq);
>
>- trace_sched_set_need_resched_tp(curr, cpu, tif);
> if (cpu == smp_processor_id()) {
> set_ti_thread_flag(cti, tif);
> if (tif == TIF_NEED_RESCHED)
> set_preempt_need_resched();
>+ trace_sched_set_need_resched_tp(curr, cpu, tif);
> return;
> }
>
>- if (set_nr_and_not_polling(cti, tif)) {
>+ send_ipi = set_nr_and_not_polling(cti, tif);
>+ trace_sched_set_need_resched_tp(curr, cpu, tif);
>+ if (send_ipi) {
> if (tif == TIF_NEED_RESCHED)
> smp_send_reschedule(cpu);
> } else {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched: Set need-resched flags before tracing
2026-09-11 21:33 [PATCH] sched: Set need-resched flags before tracing Andrea Righi
2026-09-12 7:13 ` Gabriele Monaco
@ 2026-09-12 21:30 ` bot+bpf-ci
1 sibling, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-09-12 21:30 UTC (permalink / raw)
To: arighi, mingo, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann
Cc: rostedt, bsegall, mgorman, vschneid, kprateek.nayak, gmonaco, bpf,
linux-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4927 bytes --]
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 8b3d47a325cc..ff3ce1ccf92a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2105,8 +2105,11 @@ static inline int test_tsk_thread_flag(struct task_struct *tsk, int flag)
> static inline void set_tsk_need_resched(struct task_struct *tsk)
> {
> if (tracepoint_enabled(sched_set_need_resched_tp) &&
> - !test_tsk_thread_flag(tsk, TIF_NEED_RESCHED))
> + !test_tsk_thread_flag(tsk, TIF_NEED_RESCHED)) {
> + set_tsk_thread_flag(tsk, TIF_NEED_RESCHED);
> __trace_set_need_resched(tsk, TIF_NEED_RESCHED);
> + return;
> + }
> set_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> }
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b998ef6b87af..e896d5d2c72f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1196,6 +1196,7 @@ static void __resched_curr(struct rq *rq, int tif)
> {
> struct task_struct *curr = rq->curr;
> struct thread_info *cti = task_thread_info(curr);
> + bool send_ipi;
> int cpu;
>
> lockdep_assert_rq_held(rq);
[ ... ]
> @@ -1212,15 +1213,17 @@ static void __resched_curr(struct rq *rq, int tif)
>
> cpu = cpu_of(rq);
>
> - trace_sched_set_need_resched_tp(curr, cpu, tif);
> if (cpu == smp_processor_id()) {
> set_ti_thread_flag(cti, tif);
> if (tif == TIF_NEED_RESCHED)
> set_preempt_need_resched();
> + trace_sched_set_need_resched_tp(curr, cpu, tif);
> return;
> }
>
> - if (set_nr_and_not_polling(cti, tif)) {
> + send_ipi = set_nr_and_not_polling(cti, tif);
> + trace_sched_set_need_resched_tp(curr, cpu, tif);
> + if (send_ipi) {
In the remote-CPU branch, does this ordering break the runtime
verification nrp monitor?
On TIF_POLLING_NRFLAG builds, set_nr_and_not_polling() is implemented
as:
return !(fetch_or(&ti->flags, 1 << tif) & _TIF_POLLING_NRFLAG);
so the fetch_or commits the TIF_NEED_RESCHED store before the
tracepoint's probe-list walk begins. The remote CPU can react to the
flag without waiting for any IPI:
CPU0 (holds rq1 lock) CPU1 (running target task)
----------------------------------- --------------------------
send_ipi = set_nr_and_not_polling(cti, tif)
-> target's TIF_NEED_RESCHED now set
returns from any IRQ to a
preemptible context:
raw_irqentry_exit_cond_resched()
if (need_resched() && ...)
preempt_schedule_irq();
__schedule(SM_PREEMPT)
trace_sched_entry_tp(true)
trace_sched_set_need_resched_tp(curr,...) <- arrives too late
The nrp automaton rejects schedule_entry_preempt from state
any_thread_running (the steady state after every schedule_entry).
Row any_thread_running in kernel/trace/rv/monitors/nrp/nrp.h is
{ any_thread_running, rescheduling, any_thread_running, INVALID_STATE }
so schedule_entry_preempt transitions to INVALID_STATE, printing "rv:
monitor nrp does not allow event schedule_entry_preempt on state
any_thread_running" and resetting the monitor. With the panic reactor
selected it panics the box.
The changelog says "For a remote reschedule, retain the result of
set_nr_and_not_polling() so that tracing remains ahead of IPI delivery
while still observing the updated flag." Staying ahead of the IPI is
not sufficient for correctness, because the target CPU preempts on
need_resched() alone without any IPI. raw_irqentry_exit_cond_resched()
tests need_resched() on any interrupt return, exit_to_user_mode_loop()
tests TIF_NEED_RESCHED directly, and in the polling-idle case
set_nr_and_not_polling() returns false so no IPI is ever sent while the
idle loop polls tif_need_resched().
The local-CPU branch does not have this problem: __resched_curr() runs
under the rq raw_spinlock, so preemption is disabled and rq->curr
cannot schedule before the probe returns. Likewise set_tsk_need_resched()
is safe: all three callers act on a task that cannot run concurrently
(current with IRQs disabled, or rq->idle under the rq lock).
Could the remote path emit the tracepoint before the flag store for the
remote case only (the recursion this patch fixes goes through
set_tsk_need_resched() on the local CPU, not through the remote branch
of __resched_curr()), or document why an out-of-order sched_need_resched
event is acceptable to the RV monitors?
> if (tif == TIF_NEED_RESCHED)
> smp_send_reschedule(cpu);
> } else {
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34717210328
^ permalink raw reply [flat|nested] 4+ messages in thread