Subject: Re: [RFC PATCH v3 03/10] sched: Static key to check paravirt cpu push

From: Yury Norov <[email-masked]>
Date: Wed, 10 Sep 2025 21:53:04 -0400

On Wed, Sep 10, 2025 at 11:12:03PM +0530, Shrikanth Hegde wrote:
> CPUs are marked paravirt when there is contention for underlying
> physical CPU.
> 
> The push mechanism and check for paravirt CPUs are in sched tick
> and wakeup. It should be close to no-op when there is no need for it.
> Achieve that using static key.
> 
> Architecture needs to enable this key when it decides there are
> paravirt CPUs. Disable it when there are no paravirt CPUs.

Testing a bit is quite close to a no-op, isn't it? Have you measured
the performance impact that would advocate the static key? Please
share some numbers then. I believe I asked you about it on the previous
round.

> Signed-off-by: Shrikanth Hegde <[email-masked]>
> ---
>  kernel/sched/core.c  |  1 +
>  kernel/sched/sched.h | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0f1e36bb5779..b8a84e4691c8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10967,4 +10967,5 @@ void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx)
>  #ifdef CONFIG_PARAVIRT
>  struct cpumask __cpu_paravirt_mask __read_mostly;
>  EXPORT_SYMBOL(__cpu_paravirt_mask);
> +DEFINE_STATIC_KEY_FALSE(cpu_paravirt_push_tasks);
>  #endif
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index b5367c514c14..8f9991453d36 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3880,4 +3880,21 @@ void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);
>  
>  #include "ext.h"
>  
> +#ifdef CONFIG_PARAVIRT
> +DECLARE_STATIC_KEY_FALSE(cpu_paravirt_push_tasks);
> +
> +static inline bool is_cpu_paravirt(int cpu)
> +{
> +	if (static_branch_unlikely(&cpu_paravirt_push_tasks))
> +		return cpu_paravirt(cpu);
> +
> +	return false;
> +}

So is_cpu_paravirt and cpu_paravirt are exactly the same in terms of
functionality. If you really believe that static branch benefits the
performance, it should go straight to the cpu_paravirt().

> +#else	/* !CONFIG_PARAVIRT */
> +static inline bool is_cpu_paravirt(int cpu)
> +{
> +	return false;
> +}
> +#endif	/* !CONFIG_PARAVIRT */
> +
>  #endif /* _KERNEL_SCHED_SCHED_H */
> -- 
> 2.47.3