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

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

[Vincent Guittot]

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>
---
 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

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

[Phil Auld]

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
> 
> 

-- 

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

[Peter Zijlstra]

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>

Thanks!

> @@ -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;
> +	}
> +
> +
> +}
> +
> +

I've trimmed some of that spurious whitespace.

Also, seeing this function, I'm reminded that I used to have that clamp
conditional on !CAP_SYS_NICE -- is that something we want to put in
again?

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

[Vincent Guittot]

On Fri, 10 Jan 2025 at 17:58, Peter Zijlstra <peterz@infradead.org> wrote:
>
> 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>
>
> Thanks!
>
> > @@ -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;
> > +     }
> > +
> > +
> > +}
> > +
> > +
>
> I've trimmed some of that spurious whitespace.
>
> Also, seeing this function, I'm reminded that I used to have that clamp
> conditional on !CAP_SYS_NICE -- is that something we want to put in
> again?

I don't think it's needed as I'm not sure we can handle correctly
slice outside this clamp range anyway

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

[kernel test robot]

Hi Vincent,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on linus/master v6.13-rc6 next-20250110]
[cannot apply to peterz-queue/sched/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Guittot/sched-fair-Encapsulate-set-custom-slice-in-a-__setparam_fair-function/20250110-225327
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20250110144656.484601-1-vincent.guittot%40linaro.org
patch subject: [PATCH] sched/fair: Encapsulate set custom slice in a __setparam_fair() function
config: arc-allnoconfig (https://download.01.org/0day-ci/archive/20250111/202501111759.lr8LHoZu-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250111/202501111759.lr8LHoZu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501111759.lr8LHoZu-lkp@intel.com/

All errors (new ones prefixed by >>):

   arc-elf-ld: kernel/sched/build_policy.o: in function `__sched_setscheduler':
   build_policy.c:(.text+0x41be): undefined reference to `__setparam_fair'
>> arc-elf-ld: build_policy.c:(.text+0x41be): undefined reference to `__setparam_fair'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Vincent,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on linus/master v6.13-rc6 next-20250110]
[cannot apply to peterz-queue/sched/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Guittot/sched-fair-Encapsulate-set-custom-slice-in-a-__setparam_fair-function/20250110-225327
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20250110144656.484601-1-vincent.guittot%40linaro.org
patch subject: [PATCH] sched/fair: Encapsulate set custom slice in a __setparam_fair() function
config: openrisc-alldefconfig (https://download.01.org/0day-ci/archive/20250111/202501111705.sBZc8voZ-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250111/202501111705.sBZc8voZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501111705.sBZc8voZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   or1k-linux-ld: kernel/sched/build_policy.o: in function `__sched_setscheduler':
   build_policy.c:(.text+0x8128): undefined reference to `__setparam_fair'
>> build_policy.c:(.text+0x8128): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `__setparam_fair'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[tip-bot2 for Vincent Guittot]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     2cf9ac40073ddb6a70dd5ef94f1cf45501b696ed
Gitweb:        https://git.kernel.org/tip/2cf9ac40073ddb6a70dd5ef94f1cf45501b696ed
Author:        Vincent Guittot <vincent.guittot@linaro.org>
AuthorDate:    Sat, 11 Jan 2025 10:14:09 +01:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 13 Jan 2025 14:10:22 +01:00

sched/fair: Encapsulate set custom slice in a __setparam_fair() function

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>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Phil Auld <pauld@redhat.com>
Link: https://lore.kernel.org/r/20250110144656.484601-1-vincent.guittot@linaro.org
---
 kernel/sched/fair.c     | 19 +++++++++++++++++++
 kernel/sched/sched.h    |  2 ++
 kernel/sched/syscalls.c | 16 +++-------------
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b3418b5..7ec2587 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"
@@ -5258,6 +5261,22 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq) {}
 
 #endif /* CONFIG_SMP */
 
+void __setparam_fair(struct task_struct *p, const struct sched_attr *attr)
+{
+	struct sched_entity *se = &p->se;
+
+	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 void
 place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index aef716c..300db6f 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 943406c..149e2c8 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)) {