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=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 CFDDAC433B4 for ; Tue, 18 May 2021 19:09:02 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DF5AC60E09 for ; Tue, 18 May 2021 19:09:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DF5AC60E09 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Fl5C012sKz309P for ; Wed, 19 May 2021 05:09:00 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.intel.com (client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=ricardo.neri-calderon@linux.intel.com; receiver=) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Fl5BX2GcFz2xv9 for ; Wed, 19 May 2021 05:08:34 +1000 (AEST) IronPort-SDR: GFKkQcyS0dbpU745G9hZ3yNfHkFZ8gijBl1kS1GP6fexbyFvKBqKrW6BETJYvTqSMs5I8TGCR7 v8KU7qJDc7AQ== X-IronPort-AV: E=McAfee;i="6200,9189,9988"; a="286327194" X-IronPort-AV: E=Sophos;i="5.82,310,1613462400"; d="scan'208";a="286327194" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2021 12:08:29 -0700 IronPort-SDR: E9Mhi/pl5lPivUHwoIgo5A4nYo3UxNeOuVe0huT6qSCD08qMTcTy6yf8yJHvOwqghRRIABzQJU Jp3GfO/6WoRw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.82,310,1613462400"; d="scan'208";a="439244218" Received: from ranerica-svr.sc.intel.com ([172.25.110.23]) by fmsmga008.fm.intel.com with ESMTP; 18 May 2021 12:08:29 -0700 Date: Tue, 18 May 2021 12:07:40 -0700 From: Ricardo Neri To: Peter Zijlstra Subject: Re: [PATCH v3 5/6] sched/fair: Consider SMT in ASYM_PACKING load balance Message-ID: <20210518190740.GA15251@ranerica-svr.sc.intel.com> References: <20210513154909.6385-1-ricardo.neri-calderon@linux.intel.com> <20210513154909.6385-6-ricardo.neri-calderon@linux.intel.com> <20210515021415.GB14212@ranerica-svr.sc.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210515021415.GB14212@ranerica-svr.sc.intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Len Brown , Quentin Perret , Vincent Guittot , "Ravi V. Shankar" , linux-kernel@vger.kernel.org, Aubrey Li , Daniel Bristot de Oliveira , Ricardo Neri , Steven Rostedt , Dietmar Eggemann , Ben Segall , linuxppc-dev@lists.ozlabs.org, Mel Gorman , Srinivas Pandruvada , "Joel Fernandes \(Google\)" , Tim Chen , Ingo Molnar , Aubrey Li Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, May 14, 2021 at 07:14:15PM -0700, Ricardo Neri wrote: > On Fri, May 14, 2021 at 11:47:45AM +0200, Peter Zijlstra wrote: > > On Thu, May 13, 2021 at 08:49:08AM -0700, Ricardo Neri wrote: > > > include/linux/sched/topology.h | 1 + > > > kernel/sched/fair.c | 101 +++++++++++++++++++++++++++++++++ > > > 2 files changed, 102 insertions(+) > > > > > > diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h > > > index 8f0f778b7c91..43bdb8b1e1df 100644 > > > --- a/include/linux/sched/topology.h > > > +++ b/include/linux/sched/topology.h > > > @@ -57,6 +57,7 @@ static inline int cpu_numa_flags(void) > > > #endif > > > > > > extern int arch_asym_cpu_priority(int cpu); > > > +extern bool arch_asym_check_smt_siblings(void); > > > > > > struct sched_domain_attr { > > > int relax_domain_level; > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > > index c8b66a5d593e..3d6cc027e6e6 100644 > > > --- a/kernel/sched/fair.c > > > +++ b/kernel/sched/fair.c > > > @@ -106,6 +106,15 @@ int __weak arch_asym_cpu_priority(int cpu) > > > return -cpu; > > > } > > > > > > +/* > > > + * For asym packing, first check the state of SMT siblings before deciding to > > > + * pull tasks. > > > + */ > > > +bool __weak arch_asym_check_smt_siblings(void) > > > +{ > > > + return false; > > > +} > > > + > > > /* > > > * The margin used when comparing utilization with CPU capacity. > > > * > > > > > @@ -8458,6 +8550,9 @@ sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs > > > if (group == sds->local) > > > return false; > > > > > > + if (arch_asym_check_smt_siblings()) > > > + return asym_can_pull_tasks(env->dst_cpu, sds, sgs, group); > > > + > > > return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu); > > > } > > > > So I'm thinking that this is a property of having ASYM_PACKING at a core > > level, rather than some arch special. Wouldn't something like this be > > more appropriate? > > > > --- > > --- a/include/linux/sched/topology.h > > +++ b/include/linux/sched/topology.h > > @@ -57,7 +57,6 @@ static inline int cpu_numa_flags(void) > > #endif > > > > extern int arch_asym_cpu_priority(int cpu); > > -extern bool arch_asym_check_smt_siblings(void); > > > > struct sched_domain_attr { > > int relax_domain_level; > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -107,15 +107,6 @@ int __weak arch_asym_cpu_priority(int cp > > } > > > > /* > > - * For asym packing, first check the state of SMT siblings before deciding to > > - * pull tasks. > > - */ > > -bool __weak arch_asym_check_smt_siblings(void) > > -{ > > - return false; > > -} > > - > > -/* > > * The margin used when comparing utilization with CPU capacity. > > * > > * (default: ~20%) > > @@ -8550,7 +8541,8 @@ sched_asym(struct lb_env *env, struct sd > > if (group == sds->local) > > return false; > > > > - if (arch_asym_check_smt_siblings()) > > + if ((sds->local->flags & SD_SHARE_CPUCAPACITY) || > > + (group->flags & SD_SHARE_CPUCAPACITY)) > > return asym_can_pull_tasks(env->dst_cpu, sds, sgs, group); > > Thanks Peter for the quick review! This makes sense to me. The only > reason we proposed arch_asym_check_smt_siblings() is because we were > about breaking powerpc (I need to study how they set priorities for SMT, > if applicable). If you think this is not an issue I can post a > v4 with this update. As far as I can see, priorities in powerpc are set by the CPU number. However, I am not sure how CPUs are enumerated? If CPUs in brackets are SMT sibling, Does an enumeration looks like A) [0, 1], [2, 3] or B) [0, 2], [1, 3]? I guess B is the right answer. Otherwise, both SMT siblings of a core would need to be busy before a new core is used. Still, I think the issue described in the cover letter may be reproducible in powerpc as well. If CPU3 is offlined, and [0, 2] pulled tasks from [1, -] so that both CPU0 and CPU2 become busy, CPU1 would not be able to help since CPU0 has the highest priority. I am cc'ing the linuxppc list to get some feedback. Thanks and BR, Ricardo