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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 40C98CCF9E9 for ; Thu, 26 Sep 2024 10:46:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Y3cSCZv3elvHhzPrFp9l42ChpunFrcJvxDSEUiEav1Y=; b=xozcOTxn8eA64STTQE+IFphwEi 2t2dLJeQ92HFm02LK4xu5aWRB8tONWffpREyxBvyJQrc5TDoJn+M8J/H/+ueWJW3XDz47cV19VEU/ I8furILe7dh+CeWDJYO/bqjKKo3AGbHdKZlZ/mNH8CIG8uyVzd5BQlBiCcr2zeWDJE13va0qPEiLk kKehTAA/+PrU82pw4ezhd12Rn/2WQ2My2OPpr4aLoyXtpNqt8q/5QFUl/yzXFLnfnxGoKWZ/7HLmt rgDtof8TrT6RNtSuh73NajUV2mFY4sCJe3CLWOBEucdVPk0QEyPbX+ZZfub2GyQt7104GBuQVKm33 e36BNXQQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1stm0Z-00000008794-1VKU; Thu, 26 Sep 2024 10:46:03 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1stlxp-000000086LG-1jVw for linux-arm-kernel@lists.infradead.org; Thu, 26 Sep 2024 10:43:14 +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 2A62D14BF; Thu, 26 Sep 2024 03:43:41 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 436D93F6A8; Thu, 26 Sep 2024 03:43:08 -0700 (PDT) Date: Thu, 26 Sep 2024 12:42:45 +0200 From: Beata Michalska To: Jie Zhan Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, ionela.voinescu@arm.com, sudeep.holla@arm.com, will@kernel.org, catalin.marinas@arm.com, rafael@kernel.org, viresh.kumar@linaro.org, sumitg@nvidia.com, yang@os.amperecomputing.com, vanshikonda@os.amperecomputing.com, lihuisong@huawei.com Subject: Re: [PATCH v7 1/4] cpufreq: Introduce an optional cpuinfo_avg_freq sysfs entry Message-ID: References: <20240913132944.1880703-1-beata.michalska@arm.com> <20240913132944.1880703-2-beata.michalska@arm.com> <09887c82-2813-59c3-2ff2-0b7223b37b9e@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <09887c82-2813-59c3-2ff2-0b7223b37b9e@hisilicon.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240926_034313_602794_ABBD0846 X-CRM114-Status: GOOD ( 37.64 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Sep 25, 2024 at 04:58:36PM +0800, Jie Zhan wrote: > Hi Beata, Hi Jie > > Great thanks for the update. > > On 13/09/2024 21:29, Beata Michalska wrote: > > Currently the CPUFreq core exposes two sysfs attributes that can be used > > to query current frequency of a given CPU(s): namely cpuinfo_cur_freq > > and scaling_cur_freq. Both provide slightly different view on the > > subject and they do come with their own drawbacks. > > > > cpuinfo_cur_freq provides higher precision though at a cost of being > > rather expensive. Moreover, the information retrieved via this attribute > > is somewhat short lived as frequency can change at any point of time > > making it difficult to reason from. > > > > scaling_cur_freq, on the other hand, tends to be less accurate but then > > the actual level of precision (and source of information) varies between > > architectures making it a bit ambiguous. > > > > The new attribute, cpuinfo_avg_freq, is intended to provide more stable, > > distinct interface, exposing an average frequency of a given CPU(s), as > > reported by the hardware, over a time frame spanning no more than a few > > milliseconds. As it requires appropriate hardware support, this > > interface is optional. > > > > Signed-off-by: Beata Michalska > > --- > > Documentation/admin-guide/pm/cpufreq.rst | 10 ++++++++ > > drivers/cpufreq/cpufreq.c | 31 ++++++++++++++++++++++++ > > include/linux/cpufreq.h | 1 + > > 3 files changed, 42 insertions(+) > > > > diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst > > index fe1be4ad88cb..2204d6132c05 100644 > > --- a/Documentation/admin-guide/pm/cpufreq.rst > > +++ b/Documentation/admin-guide/pm/cpufreq.rst > > @@ -248,6 +248,16 @@ are the following: > > If that frequency cannot be determined, this attribute should not > > be present. > > > > +``cpuinfo_avg_freq`` > > + An average frequency (in KHz) of all CPUs belonging to a given policy, > > + derived from a hardware provided feedback and reported on a time frame > > + spanning at most few milliseconds. > > I don't think it's necessary to put the 'at most few milliseconds' > limitation on. > > It's supposed to be fine for other platforms to implement the interface > with a longer time period, e.g. a few seconds, in the future. Otherwise, > this would probably force the implementation of 'cpuinfo_avg_freq' to be > binded with the 'scale freq tick' stuff. Actually the sched_tick was intentionally omitted from the description to avoid associating one with another. Not really sure how useful it would be to have a longer time-frames for the average frequency though. It is still intended to be rather accurate - thus the 'at most few milliseconds' statement. Extending that period reduces the accuracy. If we allow that - meaning getting average frequency over different time-frame spans , we introduce yet again platform specific behaviour for common interface, which might not be that desired. > > > + > > + This is expected to be based on the frequency the hardware actually runs > > + at and, as such, might require specialised hardware support (such as AMU > > + extension on ARM). If one cannot be determined, this attribute should > > + not be present. > > + > > ``cpuinfo_max_freq`` > > Maximum possible operating frequency the CPUs belonging to this policy > > can run at (in kHz). > > ... > > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > > index d4d2f4d1d7cb..48262073707e 100644 > > --- a/include/linux/cpufreq.h > > +++ b/include/linux/cpufreq.h > > @@ -1195,6 +1195,7 @@ static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_ > > #endif > > > > extern unsigned int arch_freq_get_on_cpu(int cpu); > > +extern int arch_freq_avg_get_on_cpu(int cpu); > > It's werid to have two different functions with mostly the same behaviour, > i.e. arch_freq_get_on_cpu() and arch_freq_avg_get_on_cpu(). > > Appreciated that there would be some capatibility work with x86 at the > moment if merging them, e.g. return type, default implementation, impact on > some userspace tools, etc. The intention here was indeed to have a clean distinction between the two. > > Anyhow, are they supposed to be merged in the near future? That depends on any further comments on that new sysfs attribute I guess. --- Thanks Beata > > > Thanks, > Jie > > > > #ifndef arch_set_freq_scale > > static __always_inline