On Mon, Mar 19, 2018 at 12:43:50PM +0000, Patrick Bellasi wrote:
On 17-Mar 20:05, Leo Yan wrote:
Function select_energy_cpu_brute() can firstly handle for case 'next_cpu == prev_cpu', so can avoid big amount code indentation under the case 'next_cpu != prev_cpu'.
The existing code is still withing the 80 cols margin and also not so bad to read.
Thus I'm not in general a big fan of this kind of function-0-change patches... and I would really like to avoid them at least when they are not part of a functional refactoring.
I'm not very used of the EAS code, still ramping up. I don't see anything shocking by setting the same indentation than the other blocks. While trying to read the code, it is more clear IMO.
Moreover...
Change-Id: Iac08b5f1e6072805c0bee22680bd0ecab411cdba Signed-off-by: Leo Yan leo.yan@linaro.org
kernel/sched/fair.c | 91 +++++++++++++++++++++++++---------------------------- 1 file changed, 42 insertions(+), 49 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 0f0b307..b67999a 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -7060,6 +7060,8 @@ static int select_energy_cpu_brute(struct task_struct *p, int prev_cpu, int sync int target_cpu; int backup_cpu; int next_cpu;
int delta = 0;
struct energy_env eenv;
schedstat_inc(p->se.statistics.nr_wakeups_secb_attempts); schedstat_inc(this_rq()->eas_stats.secb_attempts);
@@ -7108,63 +7110,54 @@ static int select_energy_cpu_brute(struct task_struct *p, int prev_cpu, int sync }
target_cpu = prev_cpu;
- if (next_cpu != prev_cpu) {
int delta = 0;
struct energy_env eenv = {
.p = p,
.util_delta = task_util(p),
/* Task's previous CPU candidate */
.cpu[EAS_CPU_PRV] = {
.cpu_id = prev_cpu,
},
/* Main alternative CPU candidate */
.cpu[EAS_CPU_NXT] = {
.cpu_id = next_cpu,
},
/* Backup alternative CPU candidate */
.cpu[EAS_CPU_BKP] = {
.cpu_id = backup_cpu,
},
};
The block above exploits the designated initializers to ensure a zero initialization to all omitted members... for example eenv.energy which is used by the following code to sum track the estimated energy of a candidate to ensure a zero initialization to all omitted members... for example eenv.energy which is used by the following code to sum track the estimated energy of a candidate.
struct energy_env eenv = {};
- if (next_cpu == prev_cpu) {
schedstat_inc(p->se.statistics.nr_wakeups_secb_count);
schedstat_inc(this_rq()->eas_stats.secb_count);
goto unlock;
- }
- eenv.p = p;
- eenv.util_delta = task_util(p);
- /* Task's previous CPU candidate */
- eenv.cpu[EAS_CPU_PRV].cpu_id = prev_cpu;
- /* Main alternative CPU candidate */
- eenv.cpu[EAS_CPU_NXT].cpu_id = next_cpu;
- /* Backup alternative CPU candidate */
- eenv.cpu[EAS_CPU_BKP].cpu_id = backup_cpu;
... AFAIKS here we don't sure eenv.cpu[XXX].energy = 0
Thus don't we get whatever is on stack as initial energy value?