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

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

[Zhan Xusheng]

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

[Peter Zijlstra]

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.

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

[kernel test robot]

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

[kernel test robot]

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

[Shubhang Kaushik Prasanna Kumar]

Hi Zhan,

The cleanup looks nice, but these helpers are sitting in some very critical hot paths. `update_stats_wait_end_fair` and `update_stats_enqueue_sleeper_fair` are hit on practically every task wakeup and context switch. 

Even though it's not a functional refactoring, have you verified it with any benchmarks? I'm concerned that introducing the helper and passing around addresses of local variables (&stats, &p) might make it harder for the compiler to optimize compared to the original version.

I'm actually looking at a similar improvement in the PELT area, so I'd be curious to know if you've seen any measurable change in overhead.. To help the compiler, it might be better to have the helper return the stats pointer directly rather than passing &stats. This avoids the double indirection and makes it easier for the compiler to keep everything in registers. Also, given how hot these paths are, using `__always_inline` would be safer than just `inline` to reduce function call overhead.

Thanks,
Shubhang Kaushik

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

[Shubhang Kaushik Prasanna Kumar]

Hi Zhan,

The cleanup looks nice, but these helpers are sitting in some very critical hot paths. `update_stats_wait_end_fair` and `update_stats_enqueue_sleeper_fair` are hit on practically every task wakeup and context switch. 

Even though it's not a functional refactoring, have you verified it with any benchmarks? I'm concerned that introducing the helper and passing around addresses of local variables (&stats, &p) might make it harder for the compiler to optimize compared to the original version.

I'm actually looking at a similar improvement in the PELT area, so I'd be curious to know if you've seen any measurable change in overhead.. To help the compiler, it might be better to have the helper return the stats pointer directly rather than passing &stats. This avoids the double indirection and makes it easier for the compiler to keep everything in registers. Also, given how hot these paths are, using `__always_inline` would be safer than just `inline` to reduce function call overhead.

Thanks,
Shubhang Kaushik

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

[Shubhang Kaushik Prasanna Kumar]

Hi Zhan,

The cleanup looks nice, but these helpers are sitting in some very critical hot paths. `update_stats_wait_end_fair` and `update_stats_enqueue_sleeper_fair` are hit on practically every task wakeup and context switch. 

Even though it's not a functional refactoring, have you verified it with any benchmarks? I'm concerned that introducing the helper and passing around addresses of local variables (&stats, &p) might make it harder for the compiler to optimize compared to the original version.

I'm actually looking at a similar improvement in the PELT area, so I'd be curious to know if you've seen any measurable change in overhead.. To help the compiler, it might be better to have the helper return the stats pointer directly rather than passing &stats. This avoids the "double indirection" and makes it easier for the compiler to keep everything in registers. Also, for these hot paths, using `__always_inline` would be safer than just `inline` to avoid possible overhead.

Thanks,
Shubhang Kaushik

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

[Shubhang Kaushik Prasanna Kumar]

Hi Zhan,

The cleanup looks nice, but these helpers are sitting in some very critical hot paths. `update_stats_wait_end_fair` and `update_stats_enqueue_sleeper_fair` are hit on practically every task wakeup and context switch. 

Even though it's not a functional refactoring, have you verified it with any benchmarks? I'm concerned that introducing the helper and passing around addresses of local variables (&stats, &p) might make it harder for the compiler to optimize compared to the original version.

I'm actually looking at a similar improvement in the PELT area, so I'd be curious to know if you've seen any measurable change in overhead.. To help the compiler, it might be better to have the helper return the stats pointer directly rather than passing &stats. This avoids the "double indirection" and makes it easier for the compiler to keep everything in registers. Also, for these hot paths, using `__always_inline` would be safer than just `inline` to avoid possible overhead.

Thanks,
Shubhang Kaushik