linux-kernel - Re: [PATCH] sched/fair: clean up asym packing

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: <7280a3b0-0727-f365-4453-8b4b01a64559@arm.com>
Date:   Mon, 3 Jun 2019 19:15:11 +0100
From:   Valentin Schneider <valentin.schneider@....com>
To:     Vincent Guittot <vincent.guittot@...aro.org>, mingo@...hat.com,
        peterz@...radead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched/fair: clean up asym packing

Hi,

On 03/06/2019 15:17, Vincent Guittot wrote:
> Clean up asym packing to follow the default load balance behavior:
> - classify the group by creating a group_asym_capacity field.

Being nitpicky here, this doesn't classify the group in the usual way
- it doesn't get a specific group_type value (group_classify()). So maybe
"classify" isn't the best term here.

Also, why tag this group in update_sd_pick_busiest()? It would make more
sense to do so in update_sg_lb_stats() like with the other sg_lb_stats fields:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 93c24473c8a0..537710026c3a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8298,6 +8298,10 @@ static inline void update_sg_lb_stats(struct lb_env *env,
                }
        }
 
+       if (sgs->sum_nr_running &&
+           sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu))
+               sgs->group_asym_capacity = 1;
+
        /* Adjust by relative CPU capacity of the group */
        sgs->group_capacity = group->sgc->capacity;
        sgs->avg_load = (sgs->group_load*SCHED_CAPACITY_SCALE) / sgs->group_capacity;
@@ -8391,9 +8395,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
         * perform better since they share less core resources.  Hence when we
         * have idle threads, we want them to be the higher ones.
         */
-       if (sgs->sum_nr_running &&
-           sched_asym_prefer(env->dst_cpu, sg->asym_prefer_cpu)) {
-               sgs->group_asym_capacity = 1;
+       if (sgs->group_asym_capacity) {
                if (!sds->busiest)
                        return true;