From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757389AbbICT62 (ORCPT ); Thu, 3 Sep 2015 15:58:28 -0400 Received: from eu-smtp-delivery-143.mimecast.com ([146.101.78.143]:11949 "EHLO eu-smtp-delivery-143.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751699AbbICT60 convert rfc822-to-8bit (ORCPT ); Thu, 3 Sep 2015 15:58:26 -0400 Subject: Re: [PATCH 2/6] sched/fair: Convert arch_scale_cpu_capacity() from weak function to #define To: Vincent Guittot , Morten Rasmussen References: <1439569394-11974-1-git-send-email-morten.rasmussen@arm.com> <1439569394-11974-3-git-send-email-morten.rasmussen@arm.com> Cc: Peter Zijlstra , "mingo@redhat.com" , Daniel Lezcano , Yuyang Du , Michael Turquette , "rjw@rjwysocki.net" , Juri Lelli , Sai Charan Gurrappadi , "pang.xunlei@zte.com.cn" , linux-kernel From: Dietmar Eggemann Message-ID: <55E8A65F.2070903@arm.com> Date: Thu, 3 Sep 2015 20:58:23 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: X-OriginalArrivalTime: 03 Sep 2015 19:58:23.0193 (UTC) FILETIME=[E392CC90:01D0E682] X-MC-Unique: CoaSrxcMT0ir8c3OC0JbEg-1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Vincent, On 02/09/15 10:31, Vincent Guittot wrote: > Hi Morten, > > On 14 August 2015 at 18:23, Morten Rasmussen wrote: >> Bring arch_scale_cpu_capacity() in line with the recent change of its >> arch_scale_freq_capacity() sibling in commit dfbca41f3479 ("sched: >> Optimize freq invariant accounting") from weak function to #define to >> allow inlining of the function. >> >> While at it, remove the ARCH_CAPACITY sched_feature as well. With the >> change to #define there isn't a straightforward way to allow runtime >> switch between an arch implementation and the default implementation of >> arch_scale_cpu_capacity() using sched_feature. The default was to use >> the arch-specific implementation, but only the arm architecture provides >> one and that is essentially equivalent to the default implementation. [...] > > So you change the way to declare arch_scale_cpu_capacity but i don't > see the update of the arm arch which declare a > arch_scale_cpu_capacity to reflect this change in your series. We were reluctant to do this because this functionality makes only sense for ARCH=arm big.Little systems w/ cortex-a{15|7} cores and only if the clock-frequency property is set in the dts file. Are you planning to push for a 'struct cpu_efficiency/clock-frequency property' solution for ARCH=arm64 as well? I'm asking because for ARCH=arm64 systems today (JUNO, Hi6220) we use the capacity value of the last entry of the capacity_state vector for the cores (e.g. cortex-a{57|53). To connect the cpu invariant engine (scale_cpu_capacity() [arch/arm/kernel/topology.c]) with the scheduler, something like this is missing: diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index 370f7a732900..17c6b3243196 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -24,6 +24,10 @@ void init_cpu_topology(void); void store_cpu_topology(unsigned int cpuid); const struct cpumask *cpu_coregroup_mask(int cpu); +#define arch_scale_cpu_capacity scale_cpu_capacity +struct sched_domain; +extern unsigned long scale_cpu_capacity(struct sched_domain *sd, int cpu); + #else static inline void init_cpu_topology(void) { } diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 08b7847bf912..907e0d2d9b82 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -42,7 +42,7 @@ */ static DEFINE_PER_CPU(unsigned long, cpu_scale); -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu) +unsigned long scale_cpu_capacity(struct sched_domain *sd, int cpu) { return per_cpu(cpu_scale, cpu); } @@ -166,7 +166,7 @@ static void update_cpu_capacity(unsigned int cpu) set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity); pr_info("CPU%u: update cpu_capacity %lu\n", - cpu, arch_scale_cpu_capacity(NULL, cpu)); + cpu, scale_cpu_capacity(NULL, cpu)); } -- Dietmar [...]