From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0AAFC6778A for ; Thu, 5 Jul 2018 14:13:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 95C2B24112 for ; Thu, 5 Jul 2018 14:13:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 95C2B24112 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753797AbeGEONy (ORCPT ); Thu, 5 Jul 2018 10:13:54 -0400 Received: from foss.arm.com ([217.140.101.70]:50540 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753418AbeGEONx (ORCPT ); Thu, 5 Jul 2018 10:13:53 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1B56218A; Thu, 5 Jul 2018 07:13:53 -0700 (PDT) Received: from e105550-lin.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 97A1B3F5BA; Thu, 5 Jul 2018 07:13:51 -0700 (PDT) Date: Thu, 5 Jul 2018 15:13:49 +0100 From: Morten Rasmussen To: Quentin Perret Cc: peterz@infradead.org, mingo@redhat.com, valentin.schneider@arm.com, dietmar.eggemann@arm.com, vincent.guittot@linaro.org, gaku.inami.xh@renesas.com, linux-kernel@vger.kernel.org Subject: Re: [PATCHv4 11/12] sched/core: Disable SD_ASYM_CPUCAPACITY for root_domains without asymmetry Message-ID: <20180705141349.GD8596@e105550-lin.cambridge.arm.com> References: <1530699470-29808-1-git-send-email-morten.rasmussen@arm.com> <1530699470-29808-12-git-send-email-morten.rasmussen@arm.com> <20180705133142.GD32579@e108498-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180705133142.GD32579@e108498-lin.cambridge.arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 05, 2018 at 02:31:43PM +0100, Quentin Perret wrote: > Hi Morten, > > On Wednesday 04 Jul 2018 at 11:17:49 (+0100), Morten Rasmussen wrote: > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c > > index 71330e0e41db..29c186961345 100644 > > --- a/kernel/sched/topology.c > > +++ b/kernel/sched/topology.c > > @@ -1160,6 +1160,26 @@ sd_init(struct sched_domain_topology_level *tl, > > sd_id = cpumask_first(sched_domain_span(sd)); > > > > /* > > + * Check if cpu_map eclipses cpu capacity asymmetry. > > + */ > > + > > + if (sd->flags & SD_ASYM_CPUCAPACITY) { > > + int i; > > + bool disable = true; > > + long capacity = arch_scale_cpu_capacity(NULL, sd_id); > > + > > + for_each_cpu(i, sched_domain_span(sd)) { > > + if (capacity != arch_scale_cpu_capacity(NULL, i)) { > > + disable = false; > > + break; > > + } > > + } > > + > > + if (disable) > > + sd->flags &= ~SD_ASYM_CPUCAPACITY; > > + } > > + > > + /* > > * Convert topological properties into behaviour. > > */ > > If SD_ASYM_CPUCAPACITY means that some CPUs have different > arch_scale_cpu_capacity() values, we could also automatically _set_ > the flag in sd_init() no ? Why should we let the arch set it and just > correct it later ? > > I understand the moment at which we know the capacities of CPUs varies > from arch to arch, but the arch code could just call > rebuild_sched_domain when the capacities of CPUs change and let the > scheduler detect things automatically. I mean, even if the arch code > sets the flag in its topology level table, it will have to rebuild > the sched domains anyway ... > > What do you think ? We could as well set the flag here so the architecture doesn't have to do it. It is a bit more complicated though for few reasons: 1. Detecting when to disable the flag is a lot simpler than checking which level is should be set on. You basically have to work you way up from the lowest topology level until you get to a level spanning all the capacities available in the system to figure out where the flag should be set. I don't think this fits easily with how we build the sched_domain hierarchy. It can of course be done. 2. As you say, we still need the arch code (or cpufreq?) to rebuild the whole thing once we know that the capacities have been determined. That currently implies implementing arch_update_cpu_topology() which is arch-specific. So we would need some arch code to make rebuild happen at the right point in time. If the rebuild should be triggering the rebuild we need another way to force a full rebuild. This can also be done. 3. Detecting the flag in generic kernel/sched/* code means that all architectures will pay the for the overhead when building/rebuilding the sched_domain hierarchy, and all architectures that sets the cpu capacities to asymmetric will set the flag whether they like it or not. I'm not sure if this is a problem. In the end it is really about how much of this we want in generic code and how much we hide in arch/, and if we dare to touch the sched_domain build code ;-) Morten