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=-8.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,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 55872C433DF for ; Mon, 3 Aug 2020 15:25:53 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 1760F206D7 for ; Mon, 3 Aug 2020 15:25:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="qtsNyqJx" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1760F206D7 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-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=6UarQ+ppGoFA5qZrmX7nmp8kZyZqiw/UD/mqzLBktVQ=; b=qtsNyqJxLMSBIvou5uLeYbFDv uc2egCaQQKHSuAKY8VHBUSi2W1uz5VsN3JgdzDTtC3NuQrBzgvbh+6GmGWiUf0kEphjEuyRei7D9o 3GiVUeOseobpOBkRwkA5phpsGIIiUGTLv5pjuGv7WsvLNALpTgvZ4fbcUlZJP4TaHRL2Fh5W5ncDS +akhTWKF0hlGWF1qUZjS5aujQIF6lHpt/Pq68miHb74jh3GMEFPXMaGhsfRf5AQo/IMWVyKAPInbR 9gEsUJwjrxo7dZYaVrzCbj8CpCxnNB3gPGYS+bOXNn5mTRnNDzcMptGusvYiEdPdcraRDsXb1VogY z52jI36cg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2cJv-0000QJ-Fs; Mon, 03 Aug 2020 15:24:11 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2cJs-0000Ps-Nq for linux-arm-kernel@lists.infradead.org; Mon, 03 Aug 2020 15:24:09 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 480B430E; Mon, 3 Aug 2020 08:24:02 -0700 (PDT) Received: from localhost (e108754-lin.cambridge.arm.com [10.1.198.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DDCC53F718; Mon, 3 Aug 2020 08:24:01 -0700 (PDT) Date: Mon, 3 Aug 2020 16:24:00 +0100 From: Ionela Voinescu To: Viresh Kumar Subject: Re: [PATCH v2 4/7] cpufreq: report whether cpufreq supports Frequency Invariance (FI) Message-ID: <20200803152400.GB20312@arm.com> References: <20200722093732.14297-1-ionela.voinescu@arm.com> <20200722093732.14297-5-ionela.voinescu@arm.com> <20200730044346.rgtaikotkgwdpc3m@vireshk-mac-ubuntu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200730044346.rgtaikotkgwdpc3m@vireshk-mac-ubuntu> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200803_112408_902983_7D3677C4 X-CRM114-Status: GOOD ( 24.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-pm@vger.kernel.org, peterz@infradead.org, catalin.marinas@arm.com, rjw@rjwysocki.net, linux@armlinux.org.uk, linux-kernel@vger.kernel.org, mingo@redhat.com, sudeep.holla@arm.com, will@kernel.org, dietmar.eggemann@arm.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Viresh, On Thursday 30 Jul 2020 at 10:13:46 (+0530), Viresh Kumar wrote: > On 22-07-20, 10:37, Ionela Voinescu wrote: > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > > index 3497c1cd6818..1d0b046fe8e9 100644 > > --- a/drivers/cpufreq/cpufreq.c > > +++ b/drivers/cpufreq/cpufreq.c > > @@ -61,6 +61,9 @@ static struct cpufreq_driver *cpufreq_driver; > > static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data); > > static DEFINE_RWLOCK(cpufreq_driver_lock); > > > > +/* Mark support for the scheduler's frequency invariance engine */ > > +static DEFINE_STATIC_KEY_FALSE(cpufreq_set_freq_scale); > > + > > /* Flag to suspend/resume CPUFreq governors */ > > static bool cpufreq_suspended; > > > > @@ -69,6 +72,25 @@ static inline bool has_target(void) > > return cpufreq_driver->target_index || cpufreq_driver->target; > > } > > > > +static inline > > +void enable_cpufreq_freq_invariance(struct cpufreq_driver *driver) > > +{ > > + if ((driver->target || driver->target_index || driver->fast_switch) && > > + !driver->setpolicy) { > > Just checking for !driver->setpolicy should be enough here. > Right, cpufreq_register_driver() should check that at least one of them is present (although currently cpufreq_register_driver() will return -EINVAL if .fast_switch() alone is present - something to be fixed). Will do, on both accounts. > > + static_branch_enable_cpuslocked(&cpufreq_set_freq_scale); > > + pr_debug("%s: Driver %s can provide frequency invariance.", > > + __func__, driver->name); > > I think a simpler print will work well too. > > pr_debug("Freq invariance enabled"); > I think the right way of reporting this support is important here. By enabling the static key here, we're not actually enabling frequency invariance. So printing "Freq invariance enabled" would be very misleading. Frequency invariance (scheduler concept) being truly enabled depends on three things: - Having a source of information for current frequency and maximum frequency (cpufreq, counters) - Having arch support for using that information to set the frequency scale factor: arch_set_freq_scale(), arch_scale_freq_tick() - Having arch support for passing the set frequency scale factor to the scheduler and reporting support for frequency invariance: arch_scale_freq_capacity(), arch_scale_freq_invariant(). Therefore, cpufreq can only report that the current driver can be a source of information for frequency invariance "Driver %s can provide frequency invariance", but it can't guarantee that the other conditions are accomplished. So I would recommend to keep this original debug message. > __func__ isn't really required as this is the only print with that > kind of info in cpufreq.c. > Makes sense! > > + } else > > + pr_err("%s: Driver %s cannot provide frequency invariance.", > > + __func__, driver->name); > > Why not supporting freq-invariance an error ? I will just drop this > message completely. Yes, an error does not make sense here. I was thinking to demote it to a warning instead in my previous reply to Rafael, but removing it altogether might be better. Many thanks for the thorough review, Ionela. > > -- > viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel