From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751625AbdEBOzA (ORCPT ); Tue, 2 May 2017 10:55:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45926 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751181AbdEBOy6 (ORCPT ); Tue, 2 May 2017 10:54:58 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5775A2062C Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lvenanci@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5775A2062C 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> <20170502144356.GB3377@worktop.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: <65a389c4-14f1-0689-47a6-5d1abfd51626@redhat.com> Date: Tue, 2 May 2017 11:54:47 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170502144356.GB3377@worktop.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.26]); Tue, 02 May 2017 14:54:58 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/02/2017 11:43 AM, Peter Zijlstra wrote: > On Fri, Apr 28, 2017 at 03:53:39PM +0200, Peter Zijlstra wrote: >> 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? >> @@ -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) > OK, so this explodes mightily. > > That code also hurt my brain bad, so I had to fix that a little. > > The below seems to boot. > > --- > --- 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)); > } > > > @@ -833,23 +834,34 @@ build_overlap_sched_groups(struct sched_ > * [*] in other words, the first group of each domain is its child domain. > */ > > -static int get_group(int cpu, struct sd_data *sdd, struct sched_group **sg) > +static struct sched_group *get_group(int cpu, struct sd_data *sdd) > { > struct sched_domain *sd = *per_cpu_ptr(sdd->sd, cpu); > struct sched_domain *child = sd->child; > + struct sched_group *sg; > > if (child) > cpu = cpumask_first(sched_domain_span(child)); > > - if (sg) { > - *sg = *per_cpu_ptr(sdd->sg, cpu); > - (*sg)->sgc = *per_cpu_ptr(sdd->sgc, cpu); > + sg = *per_cpu_ptr(sdd->sg, cpu); > + sg->sgc = *per_cpu_ptr(sdd->sgc, cpu); > > - /* For claim_allocations: */ > - atomic_set(&(*sg)->sgc->ref, 1); > + /* For claim_allocations: */ > + atomic_inc(&sg->ref); > + atomic_inc(&sg->sgc->ref); > + > + if (child) { > + cpumask_copy(sched_group_cpus(sg), sched_domain_span(child)); > + cpumask_copy(sched_group_mask(sg), sched_group_cpus(sg)); > + } else { > + cpumask_set_cpu(cpu, sched_group_cpus(sg)); > + cpumask_set_cpu(cpu, sched_group_cpus(sg)); Typo here. The mask is not being set in the else clause. > } > > - return cpu; > + sg->sgc->capacity = SCHED_CAPACITY_SCALE * cpumask_weight(sched_group_cpus(sg)); > + sg->sgc->min_capacity = SCHED_CAPACITY_SCALE; > + > + return sg; > } > > /* > @@ -868,34 +880,20 @@ build_sched_groups(struct sched_domain * > struct cpumask *covered; > int i; > > - get_group(cpu, sdd, &sd->groups); > - atomic_inc(&sd->groups->ref); > - > - if (cpu != cpumask_first(span)) > - return 0; > - > lockdep_assert_held(&sched_domains_mutex); > covered = sched_domains_tmpmask; > > cpumask_clear(covered); > > - for_each_cpu(i, span) { > + for_each_cpu_wrap(i, span, cpu) { > struct sched_group *sg; > - int group, j; > > if (cpumask_test_cpu(i, covered)) > continue; > > - group = get_group(i, sdd, &sg); > - cpumask_setall(sched_group_mask(sg)); > + sg = get_group(i, sdd); > > - for_each_cpu(j, span) { > - if (get_group(j, sdd, NULL) != group) > - continue; > - > - cpumask_set_cpu(j, covered); > - cpumask_set_cpu(j, sched_group_cpus(sg)); > - } > + cpumask_or(covered, covered, sched_group_cpus(sg)); > > if (!first) > first = sg; > @@ -904,6 +902,7 @@ build_sched_groups(struct sched_domain * > last = sg; > } > last->next = first; > + sd->groups = first; > > return 0; > }