core_rq is assigned first, so it does not need to initialize the
assignment.
Signed-off-by: Li zeming <[email protected]>
---
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
> 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
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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
On Tue, 28 May 2024 15:14:46 +0800
Li zeming <[email protected]> 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 <[email protected]>
> ---
> 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);