Re: [tip:sched/core 3/19] kernel/sched/fair.c:7263 find_energy_efficient_cpu() error: uninitialized symbol 'util_min'.

[Qais Yousef]

On 11/14/22 11:23, Dan Carpenter wrote:
> On Sat, Nov 12, 2022 at 05:47:05PM +0000, Qais Yousef wrote:
> > On 11/12/22 13:16, Dan Carpenter wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched/core
> > > head:   52b33d87b9197c51e8ffdc61873739d90dd0a16f
> > > commit: 244226035a1f9b2b6c326e55ae5188fab4f428cb [3/19] sched/uclamp: Fix fits_capacity() check in feec()
> > > config: riscv-randconfig-m031-20221111
> > > compiler: riscv64-linux-gcc (GCC) 12.1.0
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > | Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> > > 
> > > smatch warnings:
> > > kernel/sched/fair.c:7263 find_energy_efficient_cpu() error: uninitialized symbol 'util_min'.
> > > kernel/sched/fair.c:7263 find_energy_efficient_cpu() error: uninitialized symbol 'util_max'.
> > > 
> > > vim +/util_min +7263 kernel/sched/fair.c
> > 
> > [...]
> > 
> > > util_min/max not initialized if uclamp_is_used() is false.  (I thought
> > > I had reported this earlier but I don't see it on vger).
> > 
> > Thanks Dan!
> > 
> > It's harmless since util_min/max are ignored in util_fits_cpu() too;
> > uclamp_is_used() is checked there as well.
> > 
> > I couldn't reproduce, I need to get GCC 12.1.0, but the below ought to cure it.
> > Let me test it a bit more and send a patch.
> 
> In that case it's fine.  This is inlined.  I guess, generally, we are
> going to only consider passing uninitialized variables to functions as
> bugs when the functions are not inlined.  I believe that one of the
> KMsan things will trigger a warning at runtime for this as well but it's
> the same situation, where it's considered a false positive because it's
> inlined.  (Technically it's undefined behavior either way according to
> the C standard, but the standard is sometimes useless).
> 
> It's not a GCC warning, it's from Smatch.
> 
> GCC uninitialized warnings were disabled by defualt because they have
> too many false positives.  Use "make W=2" to enable them.  GCC will
> complain.

Thanks Dan. I could reproduce it a while ago and I have the full patch below
- but I couldn't run it on real hardware just to do some sanity check and it
seems I'll have to wait for a while longer.

Putting it down here in case Peter fancies picking it up. I'll be posting it
along with other fixes in this area as soon as I get chance to run them
somewhere.


Cheers

--
Qais Yousef


--->8----

commit 8539b7d7b9e3c3bb6d817cbbd5be002f80f6eb31
Author: Qais Yousef <qyousef@xxxxxxxxxxx>
Date:   Sat Nov 19 23:12:34 2022 +0000

    sched/uclamp: Fix a uninitialized variable warnings
    
    Addresses the following warnings:
    
    > config: riscv-randconfig-m031-20221111
    > compiler: riscv64-linux-gcc (GCC) 12.1.0
    >
    > smatch warnings:
    > kernel/sched/fair.c:7263 find_energy_efficient_cpu() error: uninitialized symbol 'util_min'.
    > kernel/sched/fair.c:7263 find_energy_efficient_cpu() error: uninitialized symbol 'util_max'.
    
    Reported-by: kernel test robot <lkp@xxxxxxxxx>
    Reported-by: Dan Carpenter <error27@xxxxxxxxx>
    Signed-off-by: Qais Yousef (Google) <qyousef@xxxxxxxxxxx>

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4cc56c91e06e..89dadaafc1ec 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7217,10 +7217,10 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
 	eenv_task_busy_time(&eenv, p, prev_cpu);
 
 	for (; pd; pd = pd->next) {
+		unsigned long util_min = p_util_min, util_max = p_util_max;
 		unsigned long cpu_cap, cpu_thermal_cap, util;
 		unsigned long cur_delta, max_spare_cap = 0;
 		unsigned long rq_util_min, rq_util_max;
-		unsigned long util_min, util_max;
 		unsigned long prev_spare_cap = 0;
 		int max_spare_cap_cpu = -1;
 		unsigned long base_energy;
@@ -7258,10 +7258,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
 			 * aligned with sched_cpu_util().
 			 */
 			if (uclamp_is_used()) {
-				if (uclamp_rq_is_idle(cpu_rq(cpu))) {
-					util_min = p_util_min;
-					util_max = p_util_max;
-				} else {
+				if (!uclamp_rq_is_idle(cpu_rq(cpu))) {
 					/*
 					 * Open code uclamp_rq_util_with() except for
 					 * the clamp() part. Ie: apply max aggregation