[v1] sched/fair: factor out common sched_entity stats/task lookup

[PATCH] sched/fair: factor out common sched_entity stats/task lookup

Posted by Zhan Xusheng 1 month, 3 weeks ago

The fair scheduler has several update_stats_*_fair() helpers which
open-code the same boilerplate to retrieve sched_statistics and the
associated task (if any) from a sched_entity.

Factor this common logic into a small static inline helper to reduce
duplication and improve readability, without changing behaviour or
control flow.

No functional change intended.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/sched/fair.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index da46c3164537..b4a9319a5753 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1243,6 +1243,15 @@ static void update_curr_fair(struct rq *rq)
 	update_curr(cfs_rq_of(&rq->donor->se));
 }
 
+static inline void
+get_se_stats_and_task(struct sched_entity *se,
+		struct sched_statistics **stats,
+		struct task_struct **p)
+{
+	*stats = __schedstats_from_se(se);
+	*p = entity_is_task(se) ? task_of(se) : NULL;
+}
+
 static inline void
 update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
@@ -1252,10 +1261,7 @@ update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	if (!schedstat_enabled())
 		return;
 
-	stats = __schedstats_from_se(se);
-
-	if (entity_is_task(se))
-		p = task_of(se);
+	get_se_stats_and_task(se, &stats, &p);
 
 	__update_stats_wait_start(rq_of(cfs_rq), p, stats);
 }
@@ -1269,7 +1275,7 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	if (!schedstat_enabled())
 		return;
 
-	stats = __schedstats_from_se(se);
+	get_se_stats_and_task(se, &stats, &p);
 
 	/*
 	 * When the sched_schedstat changes from 0 to 1, some sched se
@@ -1280,9 +1286,6 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	if (unlikely(!schedstat_val(stats->wait_start)))
 		return;
 
-	if (entity_is_task(se))
-		p = task_of(se);
-
 	__update_stats_wait_end(rq_of(cfs_rq), p, stats);
 }
 
@@ -1295,10 +1298,7 @@ update_stats_enqueue_sleeper_fair(struct cfs_rq *cfs_rq, struct sched_entity *se
 	if (!schedstat_enabled())
 		return;
 
-	stats = __schedstats_from_se(se);
-
-	if (entity_is_task(se))
-		tsk = task_of(se);
+	get_se_stats_and_task(se, &stats, &p);
 
 	__update_stats_enqueue_sleeper(rq_of(cfs_rq), tsk, stats);
 }
-- 
2.43.0

Re: [PATCH] sched/fair: factor out common sched_entity stats/task lookup

Posted by kernel test robot 1 month, 2 weeks ago

Hi Zhan,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on peterz-queue/sched/core linus/master v6.16-rc1 next-20251219]
[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/Zhan-Xusheng/sched-fair-factor-out-common-sched_entity-stats-task-lookup/20251217-150252
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20251217070040.8723-1-zhanxusheng%40xiaomi.com
patch subject: [PATCH] sched/fair: factor out common sched_entity stats/task lookup
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20251221/202512211954.HdIWtguJ-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512211954.HdIWtguJ-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/202512211954.HdIWtguJ-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/sched/fair.c: In function 'update_stats_enqueue_sleeper_fair':
>> kernel/sched/fair.c:1337:44: error: 'p' undeclared (first use in this function); did you mean 'up'?
    1337 |         get_se_stats_and_task(se, &stats, &p);
         |                                            ^
         |                                            up
   kernel/sched/fair.c:1337:44: note: each undeclared identifier is reported only once for each function it appears in


vim +1337 kernel/sched/fair.c

  1327	
  1328	static inline void
  1329	update_stats_enqueue_sleeper_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
  1330	{
  1331		struct sched_statistics *stats;
  1332		struct task_struct *tsk = NULL;
  1333	
  1334		if (!schedstat_enabled())
  1335			return;
  1336	
> 1337		get_se_stats_and_task(se, &stats, &p);
  1338	
  1339		__update_stats_enqueue_sleeper(rq_of(cfs_rq), tsk, stats);
  1340	}
  1341	

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

Re: [PATCH] sched/fair: factor out common sched_entity stats/task lookup

Posted by kernel test robot 1 month, 2 weeks ago

Hi Zhan,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on peterz-queue/sched/core linus/master v6.19-rc1 next-20251219]
[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/Zhan-Xusheng/sched-fair-factor-out-common-sched_entity-stats-task-lookup/20251217-150252
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20251217070040.8723-1-zhanxusheng%40xiaomi.com
patch subject: [PATCH] sched/fair: factor out common sched_entity stats/task lookup
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20251221/202512212116.MgXjzmJz-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512212116.MgXjzmJz-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/202512212116.MgXjzmJz-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/sched/fair.c:1337:37: error: use of undeclared identifier 'p'
    1337 |         get_se_stats_and_task(se, &stats, &p);
         |                                            ^
   1 error generated.


vim +/p +1337 kernel/sched/fair.c

  1327	
  1328	static inline void
  1329	update_stats_enqueue_sleeper_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
  1330	{
  1331		struct sched_statistics *stats;
  1332		struct task_struct *tsk = NULL;
  1333	
  1334		if (!schedstat_enabled())
  1335			return;
  1336	
> 1337		get_se_stats_and_task(se, &stats, &p);
  1338	
  1339		__update_stats_enqueue_sleeper(rq_of(cfs_rq), tsk, stats);
  1340	}
  1341	

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

Re: [PATCH] sched/fair: factor out common sched_entity stats/task lookup

Posted by Peter Zijlstra 1 month, 3 weeks ago

On Wed, Dec 17, 2025 at 03:00:40PM +0800, Zhan Xusheng wrote:
> The fair scheduler has several update_stats_*_fair() helpers which
> open-code the same boilerplate to retrieve sched_statistics and the
> associated task (if any) from a sched_entity.
> 
> Factor this common logic into a small static inline helper to reduce
> duplication and improve readability, without changing behaviour or
> control flow.
> 
> No functional change intended.
> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/sched/fair.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index da46c3164537..b4a9319a5753 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1243,6 +1243,15 @@ static void update_curr_fair(struct rq *rq)
>  	update_curr(cfs_rq_of(&rq->donor->se));
>  }
>  
> +static inline void
> +get_se_stats_and_task(struct sched_entity *se,
> +		struct sched_statistics **stats,
> +		struct task_struct **p)
> +{
> +	*stats = __schedstats_from_se(se);
> +	*p = entity_is_task(se) ? task_of(se) : NULL;
> +}
> +
>  static inline void
>  update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  {
> @@ -1252,10 +1261,7 @@ update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	if (!schedstat_enabled())
>  		return;
>  
> -	stats = __schedstats_from_se(se);
> -
> -	if (entity_is_task(se))
> -		p = task_of(se);
> +	get_se_stats_and_task(se, &stats, &p);
>  
>  	__update_stats_wait_start(rq_of(cfs_rq), p, stats);
>  }
> @@ -1269,7 +1275,7 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	if (!schedstat_enabled())
>  		return;
>  
> -	stats = __schedstats_from_se(se);
> +	get_se_stats_and_task(se, &stats, &p);
>  
>  	/*
>  	 * When the sched_schedstat changes from 0 to 1, some sched se
> @@ -1280,9 +1286,6 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	if (unlikely(!schedstat_val(stats->wait_start)))
>  		return;
>  
> -	if (entity_is_task(se))
> -		p = task_of(se);
> -
>  	__update_stats_wait_end(rq_of(cfs_rq), p, stats);
>  }
>  
> @@ -1295,10 +1298,7 @@ update_stats_enqueue_sleeper_fair(struct cfs_rq *cfs_rq, struct sched_entity *se
>  	if (!schedstat_enabled())
>  		return;
>  
> -	stats = __schedstats_from_se(se);
> -
> -	if (entity_is_task(se))
> -		tsk = task_of(se);
> +	get_se_stats_and_task(se, &stats, &p);
>  
>  	__update_stats_enqueue_sleeper(rq_of(cfs_rq), tsk, stats);
>  }

Urgh, no. The stats and task things are unrelated, so munging them
together like this makes no sense.

Worse, the helper is terrible for failing to use the return value for
its intended purpose.