Re: [PATCH v2 2/2] sched/fair: Fix negative energy delta in find_energy_efficient_cpu()

From: Dietmar Eggemann
Date: Mon May 03 2021 - 08:53:56 EST


On 29/04/2021 12:19, Pierre.Gondois@xxxxxxx wrote:
> From: Pierre Gondois <Pierre.Gondois@xxxxxxx>

[...]

> @@ -6680,25 +6686,23 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> }
> }
> }
> -unlock:
> +

No need for empty line.

> rcu_read_unlock();
>
> /*
> * Pick the best CPU if prev_cpu cannot be used, or if it saves at
> * least 6% of the energy used by prev_cpu.
> */
> - if (prev_delta == ULONG_MAX)
> - return best_energy_cpu;
> -
> - if ((prev_delta - best_delta) > ((prev_delta + base_energy) >> 4))
> - return best_energy_cpu;
> + if ((prev_delta == ULONG_MAX) ||
> + (prev_delta - best_delta) > ((prev_delta + base_energy) >> 4))
> + target = best_energy_cpu;

if ((prev_delta == ULONG_MAX) ||
- (prev_delta - best_delta) > ((prev_delta + base_energy) >> 4))
+ (prev_delta - best_delta) > ((prev_delta + base_energy) >> 4))
target = best_energy_cpu;

IMHO, using whitespaces to align both sub-conditions here makes it more
readable. Especially since braces aren't required around single
statements with a condition spanning over multiple lines.

[...]

With these minor things sorted:

Reviewed-by: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>