[PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq

[PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq

[Li zeming]

core_rq is assigned first, so it does not need to initialize the
assignment.

Signed-off-by: Li zeming <zeming@nfschina.com>
---
 kernel/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e32fea8f5830..346159a24705 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6485,7 +6485,7 @@ static void sched_core_cpu_starting(unsigned int cpu)
 static void sched_core_cpu_deactivate(unsigned int cpu)
 {
 	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
-	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
+	struct rq *rq = cpu_rq(cpu), *core_rq;
 	int t;
 
 	guard(core_lock)(&cpu);
-- 
2.18.2

Re: [PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq

[Markus Elfring]

> core_rq is assigned first, so it does not need to initialize the
> assignment.

* Would a wording approach (like the following) be a bit nicer?

  The variable “core_rq” will eventually be set to an appropriate pointer
  a bit later. Thus omit the explicit initialisation at the beginning.

* How do you think about to use the summary phrase
  “Delete an unnecessary initialisation in sched_core_cpu_deactivate()”?


Regards,
Markus

Re: [PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq

[kernel test robot]

Hi Li,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on peterz-queue/sched/core linus/master v6.10-rc1 next-20240528]
[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/Li-zeming/sched-core-Remove-unnecessary-NULL-values-from-core_rq/20240528-152109
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20240528071446.59197-1-zeming%40nfschina.com
patch subject: [PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq
config: s390-defconfig (https://download.01.org/0day-ci/archive/20240528/202405281844.8oxr5i5s-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project bafda89a0944d947fc4b3b5663185e07a397ac30)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240528/202405281844.8oxr5i5s-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/202405281844.8oxr5i5s-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/core.c:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2253:
   include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     500 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     501 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     507 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     508 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     519 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     520 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     528 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     529 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from kernel/sched/core.c:34:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from kernel/sched/core.c:34:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from kernel/sched/core.c:34:
   In file included from include/linux/sched/isolation.h:7:
   In file included from include/linux/tick.h:8:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> kernel/sched/core.c:6288:2: warning: variable 'core_rq' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
    6288 |         for_each_cpu(t, smt_mask) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cpumask.h:300:2: note: expanded from macro 'for_each_cpu'
     300 |         for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/find.h:586:18: note: expanded from macro 'for_each_set_bit'
     586 |         for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/sched/core.c:6295:20: note: uninitialized use occurs here
    6295 |         if (WARN_ON_ONCE(!core_rq)) /* impossible */
         |                           ^~~~~~~
   include/asm-generic/bug.h:111:25: note: expanded from macro 'WARN_ON_ONCE'
     111 |         int __ret_warn_on = !!(condition);                      \
         |                                ^~~~~~~~~
   kernel/sched/core.c:6288:2: note: remove the condition if it is always true
    6288 |         for_each_cpu(t, smt_mask) {
         |         ^
   include/linux/cpumask.h:300:2: note: expanded from macro 'for_each_cpu'
     300 |         for_each_set_bit(cpu, cpumask_bits(mask), small_cpumask_bits)
         |         ^
   include/linux/find.h:586:18: note: expanded from macro 'for_each_set_bit'
     586 |         for ((bit) = 0; (bit) = find_next_bit((addr), (size), (bit)), (bit) < (size); (bit)++)
         |                         ^
   kernel/sched/core.c:6272:39: note: initialize the variable 'core_rq' to silence this warning
    6272 |         struct rq *rq = cpu_rq(cpu), *core_rq;
         |                                              ^
         |                                               = NULL
   kernel/sched/core.c:6225:1: warning: unused function 'class_core_lock_lock_ptr' [-Wunused-function]
    6225 | DEFINE_LOCK_GUARD_1(core_lock, int,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    6226 |                     sched_core_lock(*_T->lock, &_T->flags),
         |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    6227 |                     sched_core_unlock(*_T->lock, &_T->flags),
         |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    6228 |                     unsigned long flags)
         |                     ~~~~~~~~~~~~~~~~~~~~
   include/linux/cleanup.h:232:65: note: expanded from macro 'DEFINE_LOCK_GUARD_1'
     232 | #define DEFINE_LOCK_GUARD_1(_name, _type, _lock, _unlock, ...)          \
         |                                                                         ^
     233 | __DEFINE_UNLOCK_GUARD(_name, _type, _unlock, __VA_ARGS__)               \
         | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/cleanup.h:209:21: note: expanded from macro '\
   __DEFINE_UNLOCK_GUARD'
     209 | static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T)     \
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~
   <scratch space>:76:1: note: expanded from here
      76 | class_core_lock_lock_ptr
         | ^~~~~~~~~~~~~~~~~~~~~~~~
   19 warnings generated.


vim +6288 kernel/sched/core.c

3c474b3239f12f Peter Zijlstra 2021-08-19  6268  
3c474b3239f12f Peter Zijlstra 2021-08-19  6269  static void sched_core_cpu_deactivate(unsigned int cpu)
3c474b3239f12f Peter Zijlstra 2021-08-19  6270  {
3c474b3239f12f Peter Zijlstra 2021-08-19  6271  	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
ca27ccef75e13e Li zeming      2024-05-28  6272  	struct rq *rq = cpu_rq(cpu), *core_rq;
3c474b3239f12f Peter Zijlstra 2021-08-19  6273  	int t;
3c474b3239f12f Peter Zijlstra 2021-08-19  6274  
7170509cadbb76 Peter Zijlstra 2023-08-01  6275  	guard(core_lock)(&cpu);
3c474b3239f12f Peter Zijlstra 2021-08-19  6276  
3c474b3239f12f Peter Zijlstra 2021-08-19  6277  	/* if we're the last man standing, nothing to do */
3c474b3239f12f Peter Zijlstra 2021-08-19  6278  	if (cpumask_weight(smt_mask) == 1) {
3c474b3239f12f Peter Zijlstra 2021-08-19  6279  		WARN_ON_ONCE(rq->core != rq);
7170509cadbb76 Peter Zijlstra 2023-08-01  6280  		return;
9edeaea1bc4523 Peter Zijlstra 2020-11-17  6281  	}
3c474b3239f12f Peter Zijlstra 2021-08-19  6282  
3c474b3239f12f Peter Zijlstra 2021-08-19  6283  	/* if we're not the leader, nothing to do */
3c474b3239f12f Peter Zijlstra 2021-08-19  6284  	if (rq->core != rq)
7170509cadbb76 Peter Zijlstra 2023-08-01  6285  		return;
3c474b3239f12f Peter Zijlstra 2021-08-19  6286  
3c474b3239f12f Peter Zijlstra 2021-08-19  6287  	/* find a new leader */
3c474b3239f12f Peter Zijlstra 2021-08-19 @6288  	for_each_cpu(t, smt_mask) {
3c474b3239f12f Peter Zijlstra 2021-08-19  6289  		if (t == cpu)
3c474b3239f12f Peter Zijlstra 2021-08-19  6290  			continue;
3c474b3239f12f Peter Zijlstra 2021-08-19  6291  		core_rq = cpu_rq(t);
3c474b3239f12f Peter Zijlstra 2021-08-19  6292  		break;
9edeaea1bc4523 Peter Zijlstra 2020-11-17  6293  	}
3c474b3239f12f Peter Zijlstra 2021-08-19  6294  
3c474b3239f12f Peter Zijlstra 2021-08-19  6295  	if (WARN_ON_ONCE(!core_rq)) /* impossible */
7170509cadbb76 Peter Zijlstra 2023-08-01  6296  		return;
3c474b3239f12f Peter Zijlstra 2021-08-19  6297  
3c474b3239f12f Peter Zijlstra 2021-08-19  6298  	/* copy the shared state to the new leader */
3c474b3239f12f Peter Zijlstra 2021-08-19  6299  	core_rq->core_task_seq             = rq->core_task_seq;
3c474b3239f12f Peter Zijlstra 2021-08-19  6300  	core_rq->core_pick_seq             = rq->core_pick_seq;
3c474b3239f12f Peter Zijlstra 2021-08-19  6301  	core_rq->core_cookie               = rq->core_cookie;
4feee7d12603de Josh Don       2021-10-18  6302  	core_rq->core_forceidle_count      = rq->core_forceidle_count;
3c474b3239f12f Peter Zijlstra 2021-08-19  6303  	core_rq->core_forceidle_seq        = rq->core_forceidle_seq;
4feee7d12603de Josh Don       2021-10-18  6304  	core_rq->core_forceidle_occupation = rq->core_forceidle_occupation;
4feee7d12603de Josh Don       2021-10-18  6305  
4feee7d12603de Josh Don       2021-10-18  6306  	/*
4feee7d12603de Josh Don       2021-10-18  6307  	 * Accounting edge for forced idle is handled in pick_next_task().
4feee7d12603de Josh Don       2021-10-18  6308  	 * Don't need another one here, since the hotplug thread shouldn't
4feee7d12603de Josh Don       2021-10-18  6309  	 * have a cookie.
4feee7d12603de Josh Don       2021-10-18  6310  	 */
4feee7d12603de Josh Don       2021-10-18  6311  	core_rq->core_forceidle_start = 0;
3c474b3239f12f Peter Zijlstra 2021-08-19  6312  
3c474b3239f12f Peter Zijlstra 2021-08-19  6313  	/* install new leader */
3c474b3239f12f Peter Zijlstra 2021-08-19  6314  	for_each_cpu(t, smt_mask) {
3c474b3239f12f Peter Zijlstra 2021-08-19  6315  		rq = cpu_rq(t);
3c474b3239f12f Peter Zijlstra 2021-08-19  6316  		rq->core = core_rq;
3c474b3239f12f Peter Zijlstra 2021-08-19  6317  	}
3c474b3239f12f Peter Zijlstra 2021-08-19  6318  }
3c474b3239f12f Peter Zijlstra 2021-08-19  6319  

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

Re: [PATCH] sched: core: Remove unnecessary ‘NULL’ values from core_rq

[Steven Rostedt]

On Tue, 28 May 2024 15:14:46 +0800
Li zeming <zeming@nfschina.com> wrote:

> core_rq is assigned first, so it does not need to initialize the
> assignment.

No, it is assigned in a loop. Yes, the loop should always execute once,
but the compiler doesn't know that (hence the WARN_ON() that checks
it). That means removing the NULL assignment will likely cause the
warning from the compiler that the variable may be used uninitialized.

The assignment is there at least to quiet the compiler. It's not a fast
path, and the initialization is not a problem.

NACK.

-- Steve


> 
> Signed-off-by: Li zeming <zeming@nfschina.com>
> ---
>  kernel/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e32fea8f5830..346159a24705 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6485,7 +6485,7 @@ static void sched_core_cpu_starting(unsigned int cpu)
>  static void sched_core_cpu_deactivate(unsigned int cpu)
>  {
>  	const struct cpumask *smt_mask = cpu_smt_mask(cpu);
> -	struct rq *rq = cpu_rq(cpu), *core_rq = NULL;
> +	struct rq *rq = cpu_rq(cpu), *core_rq;
>  	int t;
>  
>  	guard(core_lock)(&cpu);