mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Gabriele Monaco <gmonaco@redhat.com>,
	Sechang Lim <rhkrqnwk98@gmail.com>,
	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>,
	linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2] sched: set TIF_NEED_RESCHED before calling __trace_set_need_resched()
Date: Wed, 1 Jul 2026 10:49:39 +0200	[thread overview]
Message-ID: <20260701084939.GE751831@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <14abaec8-05ca-4034-b153-b993ed60a1fb@amd.com>

On Wed, Jul 01, 2026 at 01:39:23PM +0530, K Prateek Nayak wrote:
> Hello Peter,
> 
> On 7/1/2026 12:24 PM, Peter Zijlstra wrote:
> >> @@ -1204,16 +1217,16 @@ static void __resched_curr(struct rq *rq, int tif)
> >>  	cpu = cpu_of(rq);
> >>  
> >>  	if (cpu == smp_processor_id()) {
> >> -		set_ti_thread_flag(cti, tif);
> >> +		int set = test_and_set_ti_thread_flag(cti, tif);
> >> +
> >> +		if (trace_sched_set_need_resched_tp_enabled() && !set)
> >> +			trace_call__sched_set_need_resched_tp(curr, cpu, tif);
> >>  		if (tif == TIF_NEED_RESCHED)
> >>  			set_preempt_need_resched();
> >> -		trace_sched_set_need_resched_tp(curr, cpu, tif);
> >>  		return;
> >>  	}
> > 
> > I can't help but notice that the local and !POLLING cases show
> > remarkable similarity. Just not sure extracting that isn't going to make
> > a mess.
> > 
> > Anyway, yes this looks about right.
> 
> If the fetch_or() based path is okay for !POLLING and local cases which
> uses a slightly (vastly?) worse instruction to set the ti->flags, we can
> instead do:
> 

Tempting, but at least arm64 would have to agree I think. That said, I
wonder why ARM64 doesn't use LDXR+WFE for idle, just like we have
MONITOR+MWAIT.

Probably too damn many idle flavours to deal with or somesuch.

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index bd2f7fb87dc93..ea793e8a94a8f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1029,6 +1029,15 @@ static inline void hrtick_schedule_enter(struct rq *rq) { }
>  static inline void hrtick_schedule_exit(struct rq *rq) { }
>  #endif /* !CONFIG_SCHED_HRTICK */
>  
> +#ifndef TIF_POLLING_NRFLAG
> +/*
> + * If arch doesn't define _TIF_POLLING_NRFLAG, set it 0 to
> + * allow compilers to optimize (val & _TIF_POLLING_NRFLAG)
> + * based branches during build.
> + */
> +#define _TIF_POLLING_NRFLAG	0U
> +#endif
> +
>  /*
>   * try_cmpxchg based fetch_or() macro so it works for different integer types:
>   */
> @@ -1043,7 +1052,6 @@ static inline void hrtick_schedule_exit(struct rq *rq) { }
>  	_val;								\
>  })
>  
> -#ifdef TIF_POLLING_NRFLAG
>  /*
>   * Atomically set TIF_NEED_RESCHED and test for TIF_POLLING_NRFLAG,
>   * this avoids any races wrt polling state changes and thereby avoids
> @@ -1083,25 +1091,6 @@ static bool set_nr_if_polling(struct task_struct *p)
>  	return true;
>  }
>  
> -#else
> -static inline bool set_nr_and_not_polling(struct rq *rq, int tif)
> -{
> -	struct task_struct *curr = rq->curr;
> -	struct thread_info *ti = task_thread_info(curr);
> -	int set = test_and_set_ti_thread_flag(ti, tif);
> -
> -	if (trace_sched_set_need_resched_tp_enabled() && !set)
> -		trace_call__sched_set_need_resched_tp(curr, cpu_of(rq), tif);
> -
> -	return true;
> -}
> -
> -static inline bool set_nr_if_polling(struct task_struct *p)
> -{
> -	return false;
> -}
> -#endif
> -
>  static bool __wake_q_add(struct wake_q_head *head, struct task_struct *task)
>  {
>  	struct wake_q_node *node = &task->wake_q;
> @@ -1216,19 +1205,19 @@ static void __resched_curr(struct rq *rq, int tif)
>  
>  	cpu = cpu_of(rq);
>  
> -	if (cpu == smp_processor_id()) {
> -		int set = test_and_set_ti_thread_flag(cti, tif);
> -
> -		if (trace_sched_set_need_resched_tp_enabled() && !set)
> -			trace_call__sched_set_need_resched_tp(curr, cpu, tif);
> -		if (tif == TIF_NEED_RESCHED)
> -			set_preempt_need_resched();
> -		return;
> -	}
> -
>  	if (set_nr_and_not_polling(rq, tif)) {
> -		if (tif == TIF_NEED_RESCHED)
> -			smp_send_reschedule(cpu);
> +		if (tif != TIF_NEED_RESCHED)
> +			return;
> +		/*
> +		 * For local CPU, folding the NEED_RESCHED
> +		 * into preempt_count() is sufficient.
> +		 */
> +		if (cpu == smp_processor_id()) {
> +			set_preempt_need_resched();
> +			return;
> +		}
> +		/* Use an IPI for remote CPUs. */
> +		smp_send_reschedule(cpu);
>  	} else {
>  		trace_sched_wake_idle_without_ipi(cpu);
>  	}
> ---
> 
> set_nr_and_not_polling() will always return true for !POLLING which will
> go down the above path in __resched_curr() that deals appropriately with
> local CPU case.
> 
> set_nr_if_polling() will always return false for !POLLING from the first
> condition in the do-while loop.
> 
> Thoughts?
> 
> -- 
> Thanks and Regards,
> Prateek
> 

  reply	other threads:[~2026-07-01  8:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-27  8:16 Sechang Lim
2026-06-29  4:11 ` K Prateek Nayak
2026-06-29 12:40   ` Gabriele Monaco
2026-06-29 17:35     ` K Prateek Nayak
2026-06-30  8:58       ` Gabriele Monaco
2026-06-30 16:16         ` K Prateek Nayak
2026-06-30 20:34           ` Peter Zijlstra
2026-07-01  6:51             ` K Prateek Nayak
2026-07-01  6:54               ` Peter Zijlstra
2026-07-01  8:09                 ` K Prateek Nayak
2026-07-01  8:49                   ` Peter Zijlstra [this message]
2026-07-16 15:09                     ` Will Deacon
2026-07-16 18:25                       ` K Prateek Nayak
2026-06-30  7:58   ` Sechang Lim

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=20260701084939.GE751831@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=gmonaco@redhat.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=rhkrqnwk98@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    /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

Powered by JetHome