Re: [PATCH] sched/fair: Encapsulate set custom slice in a __setparam_fair() function

[Phil Auld <pauld@redhat.com>]

On Fri, Jan 10, 2025 at 03:46:56PM +0100 Vincent Guittot wrote:
> Similarly to dl, create a __setparam_fair() function to set parameters
> related to fair class and move it in the fair.c file.
> 
> No functional changes expected
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>

LGTM.

Reviewed-by: Phil Auld <pauld@redhat.com>

Cheers,
Phil

> ---
>  kernel/sched/fair.c     | 23 +++++++++++++++++++++++
>  kernel/sched/sched.h    |  2 ++
>  kernel/sched/syscalls.c | 16 +++-------------
>  3 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b3418b5d484f..96c7ea6fb04a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -37,6 +37,7 @@
>  #include <linux/sched/cputime.h>
>  #include <linux/sched/isolation.h>
>  #include <linux/sched/nohz.h>
> +#include <linux/sched/prio.h>
>  
>  #include <linux/cpuidle.h>
>  #include <linux/interrupt.h>
> @@ -51,6 +52,8 @@
>  
>  #include <asm/switch_to.h>
>  
> +#include <uapi/linux/sched/types.h>
> +
>  #include "sched.h"
>  #include "stats.h"
>  #include "autogroup.h"
> @@ -5050,6 +5053,26 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
>  	trace_sched_util_est_se_tp(&p->se);
>  }
>  
> +void __setparam_fair(struct task_struct *p, const struct sched_attr *attr)
> +{
> +	struct sched_entity *se = &p->se;
> +	unsigned int util_est;
> +
> +	p->static_prio = NICE_TO_PRIO(attr->sched_nice);
> +	if (attr->sched_runtime) {
> +		se->custom_slice = 1;
> +		se->slice = clamp_t(u64, attr->sched_runtime,
> +				      NSEC_PER_MSEC/10,   /* HZ=1000 * 10 */
> +				      NSEC_PER_MSEC*100); /* HZ=100  / 10 */
> +	} else {
> +		se->custom_slice = 0;
> +		se->slice = sysctl_sched_base_slice;
> +	}
> +
> +
> +}
> +
> +
>  static inline unsigned long get_actual_cpu_capacity(int cpu)
>  {
>  	unsigned long capacity = arch_scale_cpu_capacity(cpu);
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index aef716c41edb..300db6fe53eb 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3500,6 +3500,8 @@ unsigned long scale_irq_capacity(unsigned long util, unsigned long irq, unsigned
>  
>  #endif /* !CONFIG_HAVE_SCHED_AVG_IRQ */
>  
> +extern void __setparam_fair(struct task_struct *p, const struct sched_attr *attr);
> +
>  #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)
>  
>  #define perf_domain_span(pd) (to_cpumask(((pd)->em_pd->cpus)))
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index ff0e5ab4e37c..286eb72f44ae 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -300,20 +300,10 @@ static void __setscheduler_params(struct task_struct *p,
>  
>  	p->policy = policy;
>  
> -	if (dl_policy(policy)) {
> +	if (dl_policy(policy))
>  		__setparam_dl(p, attr);
> -	} else if (fair_policy(policy)) {
> -		p->static_prio = NICE_TO_PRIO(attr->sched_nice);
> -		if (attr->sched_runtime) {
> -			p->se.custom_slice = 1;
> -			p->se.slice = clamp_t(u64, attr->sched_runtime,
> -					      NSEC_PER_MSEC/10,   /* HZ=1000 * 10 */
> -					      NSEC_PER_MSEC*100); /* HZ=100  / 10 */
> -		} else {
> -			p->se.custom_slice = 0;
> -			p->se.slice = sysctl_sched_base_slice;
> -		}
> -	}
> +	else if (fair_policy(policy))
> +		__setparam_fair(p, attr);
>  
>  	/* rt-policy tasks do not have a timerslack */
>  	if (rt_or_dl_task_policy(p)) {
> -- 
> 2.43.0
> 
> 

--