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=-6.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_NEOMUTT 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 7816AC43381 for ; Mon, 18 Mar 2019 15:19:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4D3BF2085A for ; Mon, 18 Mar 2019 15:19:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727017AbfCRPTH (ORCPT ); Mon, 18 Mar 2019 11:19:07 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:36138 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726730AbfCRPTH (ORCPT ); Mon, 18 Mar 2019 11:19:07 -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 1FE851650; Mon, 18 Mar 2019 08:19:06 -0700 (PDT) Received: from e110439-lin (e110439-lin.cambridge.arm.com [10.1.194.43]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 24BFD3F59C; Mon, 18 Mar 2019 08:19:03 -0700 (PDT) Date: Mon, 18 Mar 2019 15:19:00 +0000 From: Patrick Bellasi To: Quentin Perret Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-api@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Tejun Heo , "Rafael J . Wysocki" , Vincent Guittot , Viresh Kumar , Paul Turner , Dietmar Eggemann , Morten Rasmussen , Juri Lelli , Todd Kjos , Joel Fernandes , Steve Muckle , Suren Baghdasaryan Subject: Re: [PATCH v7 10/15] sched/fair: uclamp: Add uclamp support to energy_compute() Message-ID: <20190318151900.p2lm2ys4qx7yfjhs@e110439-lin> References: <20190208100554.32196-1-patrick.bellasi@arm.com> <20190208100554.32196-11-patrick.bellasi@arm.com> <20190306172124.tpr32k6hawos7a3g@queper01-lin> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190306172124.tpr32k6hawos7a3g@queper01-lin> User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06-Mar 17:21, Quentin Perret wrote: [...] > > Since we are at that: > > - rename schedutil_freq_util() into schedutil_cpu_util(), > > since it's not only used for frequency selection. > > - use "unsigned int" instead of "unsigned long" whenever the tracked > > utilization value is not expected to overflow 32bit. > > We use unsigned long all over the place right ? All the task_util*() > functions return unsigned long, the capacity-related functions too, and > util_avg is an unsigned long in sched_avg. So I'm not sure if we want to > do this TBH. For utilization we never need more then an "unsigned int" as storage class. Even at RQ level, 32bits allows +4mln tasks. However we started with long and keep going on with that, this was just an attempt to incrementally fix that whenever we do some changes or we add some new code. But, perhaps a single whole sale update patch would fit better this job in case we really wanna do it at some point. I'll drop this change in v8 and keep this patch focused on functional bits, don't want to risk to sidetrack the discussion again. [...] > > @@ -283,13 +284,14 @@ unsigned long schedutil_freq_util(int cpu, unsigned long util_cfs, > > static unsigned long sugov_get_util(struct sugov_cpu *sg_cpu) > > { > > struct rq *rq = cpu_rq(sg_cpu->cpu); > > - unsigned long util = cpu_util_cfs(rq); > > - unsigned long max = arch_scale_cpu_capacity(NULL, sg_cpu->cpu); > > + unsigned int util_cfs = cpu_util_cfs(rq); > > + unsigned int cpu_cap = arch_scale_cpu_capacity(NULL, sg_cpu->cpu); > > Do you really need this one ? What's wrong with 'max' :-) ? Being a pretty "generic" and thus confusing name is not enough? :) Anyway, same reasoning as above and same conclusions: I'll drop the renaming so that we don't sidetrack the discussion on v8. [...] > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > index de181b8a3a2a..b9acef080d99 100644 > > --- a/kernel/sched/sched.h > > +++ b/kernel/sched/sched.h > > @@ -2335,6 +2335,7 @@ static inline unsigned long capacity_orig_of(int cpu) > > #endif > > > > #ifdef CONFIG_CPU_FREQ_GOV_SCHEDUTIL > > + > > /** > > * enum schedutil_type - CPU utilization type > > Since you're using this enum unconditionally in fair.c, you should to > move it out of the #ifdef CONFIG_CPU_FREQ_GOV_SCHEDUTIL block, I think. > > > * @FREQUENCY_UTIL: Utilization used to select frequency > > @@ -2350,15 +2351,9 @@ enum schedutil_type { > > ENERGY_UTIL, > > }; Good point, will do! Cheers, Patrick -- #include Patrick Bellasi