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.2 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=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 3599FC433DB for ; Wed, 27 Jan 2021 12:06:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EB3EB20784 for ; Wed, 27 Jan 2021 12:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237585AbhA0MGD (ORCPT ); Wed, 27 Jan 2021 07:06:03 -0500 Received: from outbound-smtp38.blacknight.com ([46.22.139.221]:57601 "EHLO outbound-smtp38.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237550AbhA0MDj (ORCPT ); Wed, 27 Jan 2021 07:03:39 -0500 Received: from mail.blacknight.com (pemlinmail06.blacknight.ie [81.17.255.152]) by outbound-smtp38.blacknight.com (Postfix) with ESMTPS id 0D45E1F0E for ; Wed, 27 Jan 2021 12:02:47 +0000 (GMT) Received: (qmail 29144 invoked from network); 27 Jan 2021 12:02:46 -0000 Received: from unknown (HELO techsingularity.net) (mgorman@techsingularity.net@[84.203.22.4]) by 81.17.254.9 with ESMTPSA (AES256-SHA encrypted, authenticated); 27 Jan 2021 12:02:46 -0000 Date: Wed, 27 Jan 2021 12:02:45 +0000 From: Mel Gorman To: Vincent Guittot Cc: Peter Zijlstra , Ingo Molnar , Li Aubrey , Qais Yousef , LKML Subject: Re: [PATCH 4/4] sched/fair: Merge select_idle_core/cpu() Message-ID: <20210127120245.GC3592@techsingularity.net> References: <20210125085909.4600-1-mgorman@techsingularity.net> <20210125085909.4600-5-mgorman@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 27, 2021 at 11:43:22AM +0100, Vincent Guittot wrote: > > @@ -6149,18 +6161,31 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > > } > > > > for_each_cpu_wrap(cpu, cpus, target) { > > - if (!--nr) > > - return -1; > > - if (available_idle_cpu(cpu) || sched_idle_cpu(cpu)) > > - break; > > + if (smt) { > > + i = select_idle_core(p, cpu, cpus, &idle_cpu); > > + if ((unsigned int)i < nr_cpumask_bits) > > + return i; > > + > > + } else { > > + if (!--nr) > > + return -1; > > + i = __select_idle_cpu(cpu); > > you should use idle_cpu directly instead of this intermediate i variable > > + idle_cpu = __select_idle_cpu(cpu); > + if ((unsigned int)idle_cpu < nr_cpumask_bits) > + break; > > Apart ths small comment above, the patch looks good to me and I > haven't any performance regression anymore > It's matching the code sequence in the SMT block. If we are going to make that change, then go the full way with this? diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 52a650aa2108..01e40e36c386 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6129,7 +6129,7 @@ static inline int select_idle_core(struct task_struct *p, int core, struct cpuma static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int target) { struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask); - int i, cpu, idle_cpu = -1, nr = INT_MAX; + int cpu, idle_cpu = -1, nr = INT_MAX; bool smt = test_idle_cores(target, false); int this = smp_processor_id(); struct sched_domain *this_sd; @@ -6162,18 +6162,16 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t for_each_cpu_wrap(cpu, cpus, target) { if (smt) { - i = select_idle_core(p, cpu, cpus, &idle_cpu); - if ((unsigned int)i < nr_cpumask_bits) - return i; + idle_cpu = select_idle_core(p, cpu, cpus, &idle_cpu); + if ((unsigned int)idle_cpu < nr_cpumask_bits) + return idle_cpu; } else { if (!--nr) return -1; - i = __select_idle_cpu(cpu); - if ((unsigned int)i < nr_cpumask_bits) { - idle_cpu = i; + idle_cpu = __select_idle_cpu(cpu); + if ((unsigned int)idle_cpu < nr_cpumask_bits) break; - } } } -- Mel Gorman SUSE Labs