linux-kernel - Re: [PATCH] sched/fair: use signed long when compute energy delta in eas

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YGLzQAvVqlrKb8AB@google.com>
Date:   Tue, 30 Mar 2021 09:45:36 +0000
From:   Quentin Perret <qperret@...gle.com>
To:     Xuewen Yan <xuewen.yan94@...il.com>
Cc:     mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, linux-kernel@...r.kernel.org,
        quentin.perret@....com, zhang.lyra@...il.com, xuewyan@...mail.com
Subject: Re: [PATCH] sched/fair: use signed long when compute energy delta in
 eas

Hi,

On Tuesday 30 Mar 2021 at 13:21:54 (+0800), Xuewen Yan wrote:
> From: Xuewen Yan <xuewen.yan@...soc.com>
> 
> now the energy delta compute as follow:
> 
> base_energy_pd = compute_energy(p, -1, pd);
> 	--->Traverse all CPUs in pd
> 	--->em_pd_energy()
> ----------------------------------------------------- \
> search for the max_sapre_cap_cpu                       \
> ---------------------------------                       search time
> cur_delta = compute_energy(p, max_spare_cap_cpu, pd);  /
> 	--->Traverse all CPUs in pd                   /
> ---------------------------------------------------- /
> 	--->em_pd_energy()
> cur_delta -= base_energy_pd;
> 
> During the search_time, or when calculate the cpu_util in
> compute_energy(), there may occurred task dequeue or cpu_util change,
> it may cause the cur_energy < base_energy_pd, so the cur_delta
> would be negative. But the cur_delta is unsigned long, at this time,
> the cur_delta would always bigger than best_delta of last pd.
> 
> Change the vars to signed long.

Is that really helping though?

Yes you will not overflow, but the decision is still 'wrong' if the util
values are not stable for the entire wake-up. I think folks on the Arm
side had patches to try and cache the util values upfront, and then use
them throughout feec() and compute_energy(), which I think would be a
better fix.

Dietmar, wdyt?

Thanks,
Quentin