Define __{set,test}_tsk_need_resched() to test for the immediacy of the
need-resched.

The current helpers, {set,test}_tsk_need_resched(...) stay the same.

In scheduler code, switch to the more explicit variants,
__set_tsk_need_resched(...), __test_tsk_need_resched(...).

Note that clear_tsk_need_resched() is only used from __schedule()
to clear the flags before switching context. Now it clears all the
need-resched flags.

Cc: Peter Ziljstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/lkml/87jzshhexi.ffs@tglx/
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 include/linux/sched.h   | 45 +++++++++++++++++++++++++++++++++++++----
 kernel/sched/core.c     |  9 +++++----
 kernel/sched/deadline.c |  4 ++--
 kernel/sched/fair.c     |  2 +-
 kernel/sched/rt.c       |  4 ++--
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 37a51115b691..804a76e6f3c5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1952,19 +1952,56 @@ static inline bool test_tsk_thread_flag(struct task_struct *tsk, int flag)
 	return test_ti_thread_flag(task_thread_info(tsk), flag);
 }
 
-static inline void set_tsk_need_resched(struct task_struct *tsk)
+/*
+ * With !CONFIG_PREEMPT_AUTO, tif_resched(RESCHED_LAZY) reduces to
+ * tif_resched(RESCHED_NOW). Add a check in the helpers below to ensure
+ * we don't touch the tif_reshed(RESCHED_NOW) bit unnecessarily.
+ */
+static inline void __set_tsk_need_resched(struct task_struct *tsk, resched_t rs)
 {
-	set_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
+	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
+		set_tsk_thread_flag(tsk, tif_resched(rs));
+	else
+		/*
+		 * RESCHED_LAZY is only touched under CONFIG_PREEMPT_AUTO.
+		 */
+		BUG();
 }
 
 static inline void clear_tsk_need_resched(struct task_struct *tsk)
 {
-	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
+	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
+
+	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
+		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
+}
+
+static inline bool __test_tsk_need_resched(struct task_struct *tsk, resched_t rs)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
+		return unlikely(test_tsk_thread_flag(tsk, tif_resched(rs)));
+	else
+		return false;
 }
 
 static inline bool test_tsk_need_resched(struct task_struct *tsk)
 {
-	return unlikely(test_tsk_thread_flag(tsk,TIF_NEED_RESCHED));
+	return __test_tsk_need_resched(tsk, RESCHED_NOW);
+}
+
+static inline bool test_tsk_need_resched_lazy(struct task_struct *tsk)
+{
+	return __test_tsk_need_resched(tsk, RESCHED_LAZY);
+}
+
+static inline void set_tsk_need_resched(struct task_struct *tsk)
+{
+	return __set_tsk_need_resched(tsk, RESCHED_NOW);
+}
+
+static inline void set_tsk_need_resched_lazy(struct task_struct *tsk)
+{
+	return __set_tsk_need_resched(tsk, RESCHED_LAZY);
 }
 
 /*
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7019a40457a6..d00d7b45303e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -933,7 +933,7 @@ static bool set_nr_if_polling(struct task_struct *p)
 #else
 static inline bool set_nr_and_not_polling(struct task_struct *p)
 {
-	set_tsk_need_resched(p);
+	__set_tsk_need_resched(p, RESCHED_NOW);
 	return true;
 }
 
@@ -1045,13 +1045,13 @@ void resched_curr(struct rq *rq)
 
 	lockdep_assert_rq_held(rq);
 
-	if (test_tsk_need_resched(curr))
+	if (__test_tsk_need_resched(curr, RESCHED_NOW))
 		return;
 
 	cpu = cpu_of(rq);
 
 	if (cpu == smp_processor_id()) {
-		set_tsk_need_resched(curr);
+		__set_tsk_need_resched(curr, RESCHED_NOW);
 		set_preempt_need_resched();
 		return;
 	}
@@ -2245,7 +2245,8 @@ void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
 	 * A queue event has occurred, and we're going to schedule.  In
 	 * this case, we can save a useless back to back clock update.
 	 */
-	if (task_on_rq_queued(rq->curr) && test_tsk_need_resched(rq->curr))
+	if (task_on_rq_queued(rq->curr) &&
+	    __test_tsk_need_resched(rq->curr, RESCHED_NOW))
 		rq_clock_skip_update(rq);
 }
 
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index a04a436af8cc..d24d6bfee293 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2035,7 +2035,7 @@ static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p,
 	 * let us try to decide what's the best thing to do...
 	 */
 	if ((p->dl.deadline == rq->curr->dl.deadline) &&
-	    !test_tsk_need_resched(rq->curr))
+	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
 		check_preempt_equal_dl(rq, p);
 #endif /* CONFIG_SMP */
 }
@@ -2564,7 +2564,7 @@ static void pull_dl_task(struct rq *this_rq)
 static void task_woken_dl(struct rq *rq, struct task_struct *p)
 {
 	if (!task_on_cpu(rq, p) &&
-	    !test_tsk_need_resched(rq->curr) &&
+	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
 	    p->nr_cpus_allowed > 1 &&
 	    dl_task(rq->curr) &&
 	    (rq->curr->nr_cpus_allowed < 2 ||
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c62805dbd608..c5171c247466 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8316,7 +8316,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
 	 * prevents us from potentially nominating it as a false LAST_BUDDY
 	 * below.
 	 */
-	if (test_tsk_need_resched(curr))
+	if (__test_tsk_need_resched(curr, RESCHED_NOW))
 		return;
 
 	/* Idle tasks are by definition preempted by non-idle tasks. */
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 3261b067b67e..f0a6c9bb890b 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1680,7 +1680,7 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
 	 * to move current somewhere else, making room for our non-migratable
 	 * task.
 	 */
-	if (p->prio == rq->curr->prio && !test_tsk_need_resched(rq->curr))
+	if (p->prio == rq->curr->prio && !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
 		check_preempt_equal_prio(rq, p);
 #endif
 }
@@ -2415,7 +2415,7 @@ static void pull_rt_task(struct rq *this_rq)
 static void task_woken_rt(struct rq *rq, struct task_struct *p)
 {
 	bool need_to_push = !task_on_cpu(rq, p) &&
-			    !test_tsk_need_resched(rq->curr) &&
+			    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
 			    p->nr_cpus_allowed > 1 &&
 			    (dl_task(rq->curr) || rt_task(rq->curr)) &&
 			    (rq->curr->nr_cpus_allowed < 2 ||
-- 
2.31.1

On Mon, May 27, 2024 at 05:34:53PM -0700, Ankur Arora wrote:

>  static inline void clear_tsk_need_resched(struct task_struct *tsk)
>  {
> -	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> +	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
> +
> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
> +		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
> +}

(using tif_resched() here is really uncalled for)

So this will generate rather sub-optimal code, namely 2 atomics that
really should be one.

Ideally we'd write this something like:

	unsigned long mask = _TIF_NEED_RESCHED;
	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
		mask |= _TIF_NEED_RESCHED_LAZY;

	atomic_long_andnot(mask, (atomic_long_t *)task_thread_info(tsk)->flags);

Which will clear both bits with a single atomic.

Peter Zijlstra <peterz@infradead.org> writes:

> On Mon, May 27, 2024 at 05:34:53PM -0700, Ankur Arora wrote:
>
>>  static inline void clear_tsk_need_resched(struct task_struct *tsk)
>>  {
>> -	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
>> +	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
>> +
>> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
>> +		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
>> +}
>
> (using tif_resched() here is really uncalled for)
>
> So this will generate rather sub-optimal code, namely 2 atomics that
> really should be one.
>
> Ideally we'd write this something like:
>
> 	unsigned long mask = _TIF_NEED_RESCHED;
> 	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
> 		mask |= _TIF_NEED_RESCHED_LAZY;
>
> 	atomic_long_andnot(mask, (atomic_long_t *)task_thread_info(tsk)->flags);
>
> Which will clear both bits with a single atomic.

Much better. Will fix.

--
ankur

On Mon, May 27, 2024 at 05:34:53PM -0700, Ankur Arora wrote:
> Define __{set,test}_tsk_need_resched() to test for the immediacy of the
> need-resched.
> 
> The current helpers, {set,test}_tsk_need_resched(...) stay the same.
> 
> In scheduler code, switch to the more explicit variants,
> __set_tsk_need_resched(...), __test_tsk_need_resched(...).
> 
> Note that clear_tsk_need_resched() is only used from __schedule()
> to clear the flags before switching context. Now it clears all the
> need-resched flags.
> 
> Cc: Peter Ziljstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Originally-by: Thomas Gleixner <tglx@linutronix.de>
> Link: https://lore.kernel.org/lkml/87jzshhexi.ffs@tglx/
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> ---
>  include/linux/sched.h   | 45 +++++++++++++++++++++++++++++++++++++----
>  kernel/sched/core.c     |  9 +++++----
>  kernel/sched/deadline.c |  4 ++--
>  kernel/sched/fair.c     |  2 +-
>  kernel/sched/rt.c       |  4 ++--
>  5 files changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 37a51115b691..804a76e6f3c5 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1952,19 +1952,56 @@ static inline bool test_tsk_thread_flag(struct task_struct *tsk, int flag)
>  	return test_ti_thread_flag(task_thread_info(tsk), flag);
>  }
>  
> -static inline void set_tsk_need_resched(struct task_struct *tsk)
> +/*
> + * With !CONFIG_PREEMPT_AUTO, tif_resched(RESCHED_LAZY) reduces to
> + * tif_resched(RESCHED_NOW). Add a check in the helpers below to ensure
> + * we don't touch the tif_reshed(RESCHED_NOW) bit unnecessarily.
> + */
> +static inline void __set_tsk_need_resched(struct task_struct *tsk, resched_t rs)
>  {
> -	set_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
> +		set_tsk_thread_flag(tsk, tif_resched(rs));
> +	else
> +		/*
> +		 * RESCHED_LAZY is only touched under CONFIG_PREEMPT_AUTO.
> +		 */
> +		BUG();
>  }

This straight up violates coding style and would require a dose of {}.

	if (!IS_ENABLED(CONFIG_PREEMPT_AUTO && rs == RESCHED_LAZY)
		BUG();

	set_tsk_thread_flag(tsk, tif_resched(rs));

seems much saner to me.

>  static inline void clear_tsk_need_resched(struct task_struct *tsk)
>  {
> -	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> +	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
> +
> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
> +		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
> +}
> +
> +static inline bool __test_tsk_need_resched(struct task_struct *tsk, resched_t rs)
> +{
> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
> +		return unlikely(test_tsk_thread_flag(tsk, tif_resched(rs)));
> +	else
> +		return false;
>  }

	if (!IS_ENABLED(CONFIG_PREEMPT_AUTO) && rs == RESCHED_LAZY)
		return false;

	return unlikely(test_tsk_thread_flag(tsk, tif_resched(rs)));

>  
>  static inline bool test_tsk_need_resched(struct task_struct *tsk)
>  {
> -	return unlikely(test_tsk_thread_flag(tsk,TIF_NEED_RESCHED));
> +	return __test_tsk_need_resched(tsk, RESCHED_NOW);
> +}
> +
> +static inline bool test_tsk_need_resched_lazy(struct task_struct *tsk)
> +{
> +	return __test_tsk_need_resched(tsk, RESCHED_LAZY);
> +}
> +
> +static inline void set_tsk_need_resched(struct task_struct *tsk)
> +{
> +	return __set_tsk_need_resched(tsk, RESCHED_NOW);
> +}
> +
> +static inline void set_tsk_need_resched_lazy(struct task_struct *tsk)
> +{
> +	return __set_tsk_need_resched(tsk, RESCHED_LAZY);
>  }
>  
>  /*

So far so good, however:

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 7019a40457a6..d00d7b45303e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -933,7 +933,7 @@ static bool set_nr_if_polling(struct task_struct *p)
>  #else
>  static inline bool set_nr_and_not_polling(struct task_struct *p)
>  {
> -	set_tsk_need_resched(p);
> +	__set_tsk_need_resched(p, RESCHED_NOW);
>  	return true;
>  }
>  
> @@ -1045,13 +1045,13 @@ void resched_curr(struct rq *rq)
>  
>  	lockdep_assert_rq_held(rq);
>  
> -	if (test_tsk_need_resched(curr))
> +	if (__test_tsk_need_resched(curr, RESCHED_NOW))
>  		return;
>  
>  	cpu = cpu_of(rq);
>  
>  	if (cpu == smp_processor_id()) {
> -		set_tsk_need_resched(curr);
> +		__set_tsk_need_resched(curr, RESCHED_NOW);
>  		set_preempt_need_resched();
>  		return;
>  	}
> @@ -2245,7 +2245,8 @@ void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
>  	 * A queue event has occurred, and we're going to schedule.  In
>  	 * this case, we can save a useless back to back clock update.
>  	 */
> -	if (task_on_rq_queued(rq->curr) && test_tsk_need_resched(rq->curr))
> +	if (task_on_rq_queued(rq->curr) &&
> +	    __test_tsk_need_resched(rq->curr, RESCHED_NOW))
>  		rq_clock_skip_update(rq);
>  }
>  
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index a04a436af8cc..d24d6bfee293 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2035,7 +2035,7 @@ static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p,
>  	 * let us try to decide what's the best thing to do...
>  	 */
>  	if ((p->dl.deadline == rq->curr->dl.deadline) &&
> -	    !test_tsk_need_resched(rq->curr))
> +	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
>  		check_preempt_equal_dl(rq, p);
>  #endif /* CONFIG_SMP */
>  }
> @@ -2564,7 +2564,7 @@ static void pull_dl_task(struct rq *this_rq)
>  static void task_woken_dl(struct rq *rq, struct task_struct *p)
>  {
>  	if (!task_on_cpu(rq, p) &&
> -	    !test_tsk_need_resched(rq->curr) &&
> +	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
>  	    p->nr_cpus_allowed > 1 &&
>  	    dl_task(rq->curr) &&
>  	    (rq->curr->nr_cpus_allowed < 2 ||
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c62805dbd608..c5171c247466 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8316,7 +8316,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>  	 * prevents us from potentially nominating it as a false LAST_BUDDY
>  	 * below.
>  	 */
> -	if (test_tsk_need_resched(curr))
> +	if (__test_tsk_need_resched(curr, RESCHED_NOW))
>  		return;
>  
>  	/* Idle tasks are by definition preempted by non-idle tasks. */
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 3261b067b67e..f0a6c9bb890b 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1680,7 +1680,7 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
>  	 * to move current somewhere else, making room for our non-migratable
>  	 * task.
>  	 */
> -	if (p->prio == rq->curr->prio && !test_tsk_need_resched(rq->curr))
> +	if (p->prio == rq->curr->prio && !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
>  		check_preempt_equal_prio(rq, p);
>  #endif
>  }
> @@ -2415,7 +2415,7 @@ static void pull_rt_task(struct rq *this_rq)
>  static void task_woken_rt(struct rq *rq, struct task_struct *p)
>  {
>  	bool need_to_push = !task_on_cpu(rq, p) &&
> -			    !test_tsk_need_resched(rq->curr) &&
> +			    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
>  			    p->nr_cpus_allowed > 1 &&
>  			    (dl_task(rq->curr) || rt_task(rq->curr)) &&
>  			    (rq->curr->nr_cpus_allowed < 2 ||

These are all NO-OPs.... Changelog says:
 
> In scheduler code, switch to the more explicit variants,
> __set_tsk_need_resched(...), __test_tsk_need_resched(...).

But leaves me wondering *WHY* ?!?

I can't help but feel this patch attempts to do 2 things and fails to
justify at least one of them.

Peter Zijlstra <peterz@infradead.org> writes:

> On Mon, May 27, 2024 at 05:34:53PM -0700, Ankur Arora wrote:
>> Define __{set,test}_tsk_need_resched() to test for the immediacy of the
>> need-resched.
>>
>> The current helpers, {set,test}_tsk_need_resched(...) stay the same.
>>
>> In scheduler code, switch to the more explicit variants,
>> __set_tsk_need_resched(...), __test_tsk_need_resched(...).
>>
>> Note that clear_tsk_need_resched() is only used from __schedule()
>> to clear the flags before switching context. Now it clears all the
>> need-resched flags.
>>
>> Cc: Peter Ziljstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Juri Lelli <juri.lelli@redhat.com>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Paul E. McKenney <paulmck@kernel.org>
>> Originally-by: Thomas Gleixner <tglx@linutronix.de>
>> Link: https://lore.kernel.org/lkml/87jzshhexi.ffs@tglx/
>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>> ---
>>  include/linux/sched.h   | 45 +++++++++++++++++++++++++++++++++++++----
>>  kernel/sched/core.c     |  9 +++++----
>>  kernel/sched/deadline.c |  4 ++--
>>  kernel/sched/fair.c     |  2 +-
>>  kernel/sched/rt.c       |  4 ++--
>>  5 files changed, 51 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 37a51115b691..804a76e6f3c5 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1952,19 +1952,56 @@ static inline bool test_tsk_thread_flag(struct task_struct *tsk, int flag)
>>  	return test_ti_thread_flag(task_thread_info(tsk), flag);
>>  }
>>
>> -static inline void set_tsk_need_resched(struct task_struct *tsk)
>> +/*
>> + * With !CONFIG_PREEMPT_AUTO, tif_resched(RESCHED_LAZY) reduces to
>> + * tif_resched(RESCHED_NOW). Add a check in the helpers below to ensure
>> + * we don't touch the tif_reshed(RESCHED_NOW) bit unnecessarily.
>> + */
>> +static inline void __set_tsk_need_resched(struct task_struct *tsk, resched_t rs)
>>  {
>> -	set_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
>> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
>> +		set_tsk_thread_flag(tsk, tif_resched(rs));
>> +	else
>> +		/*
>> +		 * RESCHED_LAZY is only touched under CONFIG_PREEMPT_AUTO.
>> +		 */
>> +		BUG();
>>  }
>
> This straight up violates coding style and would require a dose of {}.
>
> 	if (!IS_ENABLED(CONFIG_PREEMPT_AUTO && rs == RESCHED_LAZY)
> 		BUG();
>
> 	set_tsk_thread_flag(tsk, tif_resched(rs));
>
> seems much saner to me.
>
>>  static inline void clear_tsk_need_resched(struct task_struct *tsk)
>>  {
>> -	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
>> +	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
>> +
>> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
>> +		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
>> +}
>> +
>> +static inline bool __test_tsk_need_resched(struct task_struct *tsk, resched_t rs)
>> +{
>> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO) || rs == RESCHED_NOW)
>> +		return unlikely(test_tsk_thread_flag(tsk, tif_resched(rs)));
>> +	else
>> +		return false;
>>  }
>
> 	if (!IS_ENABLED(CONFIG_PREEMPT_AUTO) && rs == RESCHED_LAZY)
> 		return false;
>
> 	return unlikely(test_tsk_thread_flag(tsk, tif_resched(rs)));
>
>>
>>  static inline bool test_tsk_need_resched(struct task_struct *tsk)
>>  {
>> -	return unlikely(test_tsk_thread_flag(tsk,TIF_NEED_RESCHED));
>> +	return __test_tsk_need_resched(tsk, RESCHED_NOW);
>> +}
>> +
>> +static inline bool test_tsk_need_resched_lazy(struct task_struct *tsk)
>> +{
>> +	return __test_tsk_need_resched(tsk, RESCHED_LAZY);
>> +}
>> +
>> +static inline void set_tsk_need_resched(struct task_struct *tsk)
>> +{
>> +	return __set_tsk_need_resched(tsk, RESCHED_NOW);
>> +}
>> +
>> +static inline void set_tsk_need_resched_lazy(struct task_struct *tsk)
>> +{
>> +	return __set_tsk_need_resched(tsk, RESCHED_LAZY);
>>  }
>>
>>  /*
>
> So far so good, however:
>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 7019a40457a6..d00d7b45303e 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -933,7 +933,7 @@ static bool set_nr_if_polling(struct task_struct *p)
>>  #else
>>  static inline bool set_nr_and_not_polling(struct task_struct *p)
>>  {
>> -	set_tsk_need_resched(p);
>> +	__set_tsk_need_resched(p, RESCHED_NOW);
>>  	return true;
>>  }
>>
>> @@ -1045,13 +1045,13 @@ void resched_curr(struct rq *rq)
>>
>>  	lockdep_assert_rq_held(rq);
>>
>> -	if (test_tsk_need_resched(curr))
>> +	if (__test_tsk_need_resched(curr, RESCHED_NOW))
>>  		return;
>>
>>  	cpu = cpu_of(rq);
>>
>>  	if (cpu == smp_processor_id()) {
>> -		set_tsk_need_resched(curr);
>> +		__set_tsk_need_resched(curr, RESCHED_NOW);
>>  		set_preempt_need_resched();
>>  		return;
>>  	}
>> @@ -2245,7 +2245,8 @@ void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
>>  	 * A queue event has occurred, and we're going to schedule.  In
>>  	 * this case, we can save a useless back to back clock update.
>>  	 */
>> -	if (task_on_rq_queued(rq->curr) && test_tsk_need_resched(rq->curr))
>> +	if (task_on_rq_queued(rq->curr) &&
>> +	    __test_tsk_need_resched(rq->curr, RESCHED_NOW))
>>  		rq_clock_skip_update(rq);
>>  }
>>
>> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>> index a04a436af8cc..d24d6bfee293 100644
>> --- a/kernel/sched/deadline.c
>> +++ b/kernel/sched/deadline.c
>> @@ -2035,7 +2035,7 @@ static void wakeup_preempt_dl(struct rq *rq, struct task_struct *p,
>>  	 * let us try to decide what's the best thing to do...
>>  	 */
>>  	if ((p->dl.deadline == rq->curr->dl.deadline) &&
>> -	    !test_tsk_need_resched(rq->curr))
>> +	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
>>  		check_preempt_equal_dl(rq, p);
>>  #endif /* CONFIG_SMP */
>>  }
>> @@ -2564,7 +2564,7 @@ static void pull_dl_task(struct rq *this_rq)
>>  static void task_woken_dl(struct rq *rq, struct task_struct *p)
>>  {
>>  	if (!task_on_cpu(rq, p) &&
>> -	    !test_tsk_need_resched(rq->curr) &&
>> +	    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
>>  	    p->nr_cpus_allowed > 1 &&
>>  	    dl_task(rq->curr) &&
>>  	    (rq->curr->nr_cpus_allowed < 2 ||
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index c62805dbd608..c5171c247466 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8316,7 +8316,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>>  	 * prevents us from potentially nominating it as a false LAST_BUDDY
>>  	 * below.
>>  	 */
>> -	if (test_tsk_need_resched(curr))
>> +	if (__test_tsk_need_resched(curr, RESCHED_NOW))
>>  		return;
>>
>>  	/* Idle tasks are by definition preempted by non-idle tasks. */
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 3261b067b67e..f0a6c9bb890b 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1680,7 +1680,7 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
>>  	 * to move current somewhere else, making room for our non-migratable
>>  	 * task.
>>  	 */
>> -	if (p->prio == rq->curr->prio && !test_tsk_need_resched(rq->curr))
>> +	if (p->prio == rq->curr->prio && !__test_tsk_need_resched(rq->curr, RESCHED_NOW))
>>  		check_preempt_equal_prio(rq, p);
>>  #endif
>>  }
>> @@ -2415,7 +2415,7 @@ static void pull_rt_task(struct rq *this_rq)
>>  static void task_woken_rt(struct rq *rq, struct task_struct *p)
>>  {
>>  	bool need_to_push = !task_on_cpu(rq, p) &&
>> -			    !test_tsk_need_resched(rq->curr) &&
>> +			    !__test_tsk_need_resched(rq->curr, RESCHED_NOW) &&
>>  			    p->nr_cpus_allowed > 1 &&
>>  			    (dl_task(rq->curr) || rt_task(rq->curr)) &&
>>  			    (rq->curr->nr_cpus_allowed < 2 ||
>
> These are all NO-OPs.... Changelog says:
>
>> In scheduler code, switch to the more explicit variants,
>> __set_tsk_need_resched(...), __test_tsk_need_resched(...).
>
> But leaves me wondering *WHY* ?!?
>
> I can't help but feel this patch attempts to do 2 things and fails to
> justify at least one of them.

So, yes all of the scheduler changes are NOP. In later patches the
scheduler will care about specifying the specific type of resched_t
and so cannot use need_resched() or need_resched_lazy().

Changed that here to minimize the interface change noise later-on.

Does something like the following help justify why they should be here?

  In scheduler code, switch to the more explicit variants,
  __set_tsk_need_resched(...), __test_tsk_need_resched(...) as a
  preparatory step for PREEMPT_AUTO support.

Thanks for all the comments, btw.

--
ankur