From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1173152AbdDXPMS (ORCPT ); Mon, 24 Apr 2017 11:12:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38894 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1046362AbdDXPMJ (ORCPT ); Mon, 24 Apr 2017 11:12:09 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E42AE8051F Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lvenanci@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E42AE8051F Reply-To: lvenanci@redhat.com Subject: Re: [PATCH 4/4] sched/topology: the group balance cpu must be a cpu where the group is installed References: <1492717903-5195-1-git-send-email-lvenanci@redhat.com> <1492717903-5195-5-git-send-email-lvenanci@redhat.com> <20170424130326.nfbaujvcdjca22tl@hirez.programming.kicks-ass.net> To: Peter Zijlstra Cc: lwang@redhat.com, riel@redhat.com, Mike Galbraith , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org From: Lauro Venancio Organization: Red Hat Message-ID: <5c03e2ca-e67a-f3e0-2a93-c91ba0f11973@redhat.com> Date: Mon, 24 Apr 2017 12:11:59 -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: <20170424130326.nfbaujvcdjca22tl@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.27]); Mon, 24 Apr 2017 15:12:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/24/2017 10:03 AM, Peter Zijlstra wrote: > On Thu, Apr 20, 2017 at 04:51:43PM -0300, Lauro Ramos Venancio wrote: > >> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c >> index e77c93a..694e799 100644 >> --- a/kernel/sched/topology.c >> +++ b/kernel/sched/topology.c >> @@ -505,7 +507,11 @@ static void build_group_mask(struct sched_domain *sd, struct sched_group *sg) >> >> for_each_cpu(i, sg_span) { >> sibling = *per_cpu_ptr(sdd->sd, i); >> - if (!cpumask_test_cpu(i, sched_domain_span(sibling))) >> + >> + if (!sibling->groups) >> + continue; > How can this happen? This happens on machines with asymmetric topologies. For more details see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c1174876874dcf8986806e4dad3d7d07af20b439 > >> + >> + if (!cpumask_equal(sg_span, sched_group_cpus(sibling->groups))) >> continue; >> >> cpumask_set_cpu(i, sched_group_mask(sg)); > >> @@ -1482,6 +1502,14 @@ struct sched_domain *build_sched_domain(struct sched_domain_topology_level *tl, >> } >> } >> >> + /* Init overlap groups */ >> + for_each_cpu(i, cpu_map) { >> + for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) { >> + if (sd->flags & SD_OVERLAP) >> + init_overlap_sched_groups(sd); >> + } >> + } > Why does this have to be a whole new loop? This is because in > build_group_mask() we could encounter @sibling that were not constructed > yet? That is right. We can only build the group mask when all siblings are constructed. > > So this is the primary fix? Yes. > >> + >> /* Calculate CPU capacity for physical packages and nodes */ >> for (i = nr_cpumask_bits-1; i >= 0; i--) { >> if (!cpumask_test_cpu(i, cpu_map)) > > Also, would it not make sense to re-order patch 2 to come after this, > such that we _do_ have the group_mask available and don't have to jump > through hoops in order to link up the sgc? Afaict we don't actually use > the sgc until the above (reverse) loop computing the CPU capacities.