From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1425193AbdD1XaY (ORCPT ); Fri, 28 Apr 2017 19:30:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32828 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753520AbdD1XaP (ORCPT ); Fri, 28 Apr 2017 19:30:15 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4863AC054C5C Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lvenanci@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 4863AC054C5C Reply-To: lvenanci@redhat.com Subject: Re: [PATCH 00/14] sched/topology fixes References: <20170428131958.893188882@infradead.org> <20170428135339.diwcabxhcpu4b5fw@hirez.programming.kicks-ass.net> To: Peter Zijlstra , mingo@kernel.org Cc: lwang@redhat.com, riel@redhat.com, efault@gmx.de, tglx@linutronix.de, linux-kernel@vger.kernel.org From: Lauro Venancio Organization: Red Hat Message-ID: <8905892c-a5b8-3a40-87e6-e0c33e4ca9bc@redhat.com> Date: Fri, 28 Apr 2017 20:30:05 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170428135339.diwcabxhcpu4b5fw@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 28 Apr 2017 23:30:14 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/28/2017 10:53 AM, Peter Zijlstra wrote: > On Fri, Apr 28, 2017 at 03:19:58PM +0200, Peter Zijlstra wrote: >> Hi, >> >> These patches are based upon the hard work of Lauro. He put in the time and >> effort to understand and debug the code. >> >> So while I didn't take many of his actual patches; I want to thank him for >> doing the work. Hopefully the "Debugged-by:" tag conveys some of that. >> >> In any case, please have a look. I think these should about cover things. >> >> Rik, Lauro, could you guys in particular look at the final patch that adds a >> few comments. I attempted to document the intent and understanding there. But >> given I've been staring at this stuff too long I could've missed the obvious. >> >> Comments and or suggestions welcome. >> > Also, the following occurred to me: > > sg_span & sg_mask == sg_mask > > Therefore, we don't need to do the whole "sg_span &" business. > > Hmm? Agreed. Maybe we have to rename "mask" to something else. Maybe "group_reached_by_cpus" or "group_installed_on_cpus". > > --- > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -7996,7 +7996,7 @@ static int active_load_balance_cpu_stop( > static int should_we_balance(struct lb_env *env) > { > struct sched_group *sg = env->sd->groups; > - struct cpumask *sg_cpus, *sg_mask; > + struct cpumask *sg_mask; > int cpu, balance_cpu = -1; > > /* > @@ -8006,11 +8006,10 @@ static int should_we_balance(struct lb_e > if (env->idle == CPU_NEWLY_IDLE) > return 1; > > - sg_cpus = sched_group_cpus(sg); > sg_mask = sched_group_mask(sg); > /* Try to find first idle cpu */ > - for_each_cpu_and(cpu, sg_cpus, env->cpus) { > - if (!cpumask_test_cpu(cpu, sg_mask) || !idle_cpu(cpu)) > + for_each_cpu_and(cpu, sg_mask, env->cpus) { > + if (!idle_cpu(cpu)) > continue; > > balance_cpu = cpu; > --- a/kernel/sched/topology.c > +++ b/kernel/sched/topology.c > @@ -85,7 +85,8 @@ static int sched_domain_debug_one(struct > group->sgc->id, > cpumask_pr_args(sched_group_cpus(group))); > > - if ((sd->flags & SD_OVERLAP) && !cpumask_full(sched_group_mask(group))) { > + if ((sd->flags & SD_OVERLAP) && > + !cpumask_equal(sched_group_mask(group), sched_group_cpus(group))) { > printk(KERN_CONT " mask=%*pbl", > cpumask_pr_args(sched_group_mask(group))); > } > @@ -505,7 +506,7 @@ enum s_alloc { > */ > int group_balance_cpu(struct sched_group *sg) > { > - return cpumask_first_and(sched_group_cpus(sg), sched_group_mask(sg)); > + return cpumask_first(sched_group_mask(sg)); > } > > > @@ -856,7 +857,7 @@ build_sched_groups(struct sched_domain * > continue; > > group = get_group(i, sdd, &sg); > - cpumask_setall(sched_group_mask(sg)); > + cpumask_copy(sched_group_mask(sg), sched_group_cpus(sg)); > > for_each_cpu(j, span) { > if (get_group(j, sdd, NULL) != group)