Received-SPF: pass (google.com: domain of linux-kernel+bounces-199336-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <fa715df1-bc7f-4abc-8554-947484b2c0ae@linux.ibm.com>
Date: Mon, 3 Jun 2024 20:34:46 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 04/35] preempt: introduce CONFIG_PREEMPT_AUTO
To: Ankur Arora <ankur.a.arora@oracle.com>
Cc: tglx@linutronix.de, peterz@infradead.org, torvalds@linux-foundation.org,
        paulmck@kernel.org, rostedt@goodmis.org, mark.rutland@arm.com,
        juri.lelli@redhat.com, joel@joelfernandes.org, raghavendra.kt@amd.com,
        boris.ostrovsky@oracle.com, konrad.wilk@oracle.com,
        Jonathan Corbet <corbet@lwn.net>, LKML <linux-kernel@vger.kernel.org>
References: <20240528003521.979836-1-ankur.a.arora@oracle.com>
 <20240528003521.979836-5-ankur.a.arora@oracle.com>
From: Shrikanth Hegde <sshegde@linux.ibm.com>
Content-Language: en-US
In-Reply-To: <20240528003521.979836-5-ankur.a.arora@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0


On 5/28/24 6:04 AM, Ankur Arora wrote:
> PREEMPT_AUTO adds a new scheduling model which, like PREEMPT_DYNAMIC,
> allows dynamic switching between a none/voluntary/full preemption
> model. However, unlike PREEMPT_DYNAMIC, it doesn't use explicit
> preemption points for the voluntary models.
> 
> It works by depending on CONFIG_PREEMPTION (and thus PREEMPT_COUNT),
> allowing the scheduler to always know when it is safe to preempt
> for all three preemption models.
> 
> In addition, it uses an additional need-resched bit
> (TIF_NEED_RESCHED_LAZY) which, with TIF_NEED_RESCHED allows the
> scheduler to express two kinds of rescheduling intent: schedule at
> the earliest opportunity (the usual TIF_NEED_RESCHED semantics), or
> express a need for rescheduling while allowing the task on the
> runqueue to run to timeslice completion (TIF_NEED_RESCHED_LAZY).
> 
> The scheduler chooses the specific need-resched flag based on
> the preemption model:
> 
> 		TIF_NEED_RESCHED 	TIF_NEED_RESCHED_LAZY
> 
> none		never   		always [*]
> voluntary       higher sched class	other tasks [*]
> full 		always                  never
> 
> [*] when preempting idle, or for kernel tasks that are 'urgent' in
> some way (ex. resched_cpu() used as an RCU hammer), we use
> TIF_NEED_RESCHED.
> 
> The other part is when preemption happens -- or, when are the
> need-resched flags checked:
> 
>                  exit-to-user    ret-to-kernel    preempt_count()
> NEED_RESCHED_LAZY     Y               N                N
> NEED_RESCHED          Y               Y                Y
> 
> Exposed under CONFIG_EXPERT for now.
> 
> Cc: Peter Ziljstra <peterz@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> 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>
> ---
>  .../admin-guide/kernel-parameters.txt         |  1 +
>  include/linux/thread_info.h                   | 12 ++++++
>  init/Makefile                                 |  1 +
>  kernel/Kconfig.preempt                        | 37 +++++++++++++++++--
>  4 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2d693300ab57..16a91090d167 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4719,6 +4719,7 @@
>  
>  	preempt=	[KNL]
>  			Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC
> +			or CONFIG_PREEMPT_AUTO.
>  			none - Limited to cond_resched() calls
>  			voluntary - Limited to cond_resched() and might_sleep() calls
>  			full - Any section that isn't explicitly preempt disabled
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index 9ea0b28068f4..06e13e7acbe2 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -59,6 +59,18 @@ enum syscall_work_bit {
>  
>  #include <asm/thread_info.h>
>  
> +/*
> + * Fall back to the default need-resched flag when an architecture does not
> + * define TIF_NEED_RESCHED_LAZY.
> + *
> + * Note: with !PREEMPT_AUTO, code should not be setting TIF_NEED_RESCHED_LAZY
> + * anywhere. Define this here because we will explicitly test for this bit.
> + */


Is this comment still valid? 
I see that flag has been set without any checks in arch file. 


> +#ifndef TIF_NEED_RESCHED_LAZY
> +#define TIF_NEED_RESCHED_LAZY TIF_NEED_RESCHED
> +#define _TIF_NEED_RESCHED_LAZY _TIF_NEED_RESCHED
> +#endif
> +
>  #ifdef __KERNEL__
>  
>  #ifndef arch_set_restart_data