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 498C8CE7AFD for ; Fri, 6 Sep 2024 10:07:59 +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=TjwTgn+mKO9TrmGYz0/O+qyZfViC+ENS4hvVLJdUf+I=; b=UUvP5dbnuiaVL5JsMm6uMBI15R EmNL2ATCWDjo72pvRlVLYrW54K5luXlQf3hfmsnzZmKcR+pUsoIsoWQ0uCG4T9tZVclobtwcKPB+p 7MtCmhK4MshRHQfgmZiko4FQBgs0PtiyaqODWDtRXyCOrScFKHWBLEWGq86CQfhrGiet5NLNfVtaN mnwUKylGoxcPuqayhy3ifxxD34yH/91vw23SWQxMZEt2fvKeuUh1wQLXULqswxf+cM4wvNs615rUw ZhmGzkh8RXdLbvDbWXvNkqrwrRom0/sA0lnUtWF3qtAcOrqTMEHI74hEN4FWy0InVgUTUQYK5JMwc PKcmbGjQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1smVsZ-0000000BgiX-0eNV; Fri, 06 Sep 2024 10:07:47 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1smVXi-0000000BajA-22r7 for linux-arm-kernel@lists.infradead.org; Fri, 06 Sep 2024 09:46:16 +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 38671FEC; Fri, 6 Sep 2024 02:46:39 -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 BBDD33F73F; Fri, 6 Sep 2024 02:46:08 -0700 (PDT) Date: Fri, 6 Sep 2024 11:45:49 +0200 From: Beata Michalska To: Jie Zhan Cc: Catalin Marinas , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, ionela.voinescu@arm.com, sudeep.holla@arm.com, will@kernel.org, vincent.guittot@linaro.org, vanshikonda@os.amperecomputing.com, sumitg@nvidia.com, yang@os.amperecomputing.com, lihuisong@huawei.com, viresh.kumar@linaro.org, rafael@kernel.org Subject: Re: [PATCH v6 0/4] Add support for AArch64 AMUv1-based arch_freq_get_on_cpu Message-ID: References: <20240603082154.3830591-1-beata.michalska@arm.com> <8500d58c-e6c5-04c7-73a0-38d3f77f2cb7@hisilicon.com> <8a9b4e02-a5c6-cb1b-fd32-728fc2c5e741@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8a9b4e02-a5c6-cb1b-fd32-728fc2c5e741@hisilicon.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240906_024614_632188_09AE2AB8 X-CRM114-Status: GOOD ( 35.61 ) 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 Tue, Aug 27, 2024 at 08:56:28PM +0800, Jie Zhan wrote: > On 26/08/2024 15:24, Beata Michalska wrote: > > ... > > > > I've recently tested this patchset on a Kunpeng system. > > > It works as expected when reading scaling_cur_freq. > > > The frequency samples are much stabler than what cppc_cpufreq returns. > > Thank you for giving it a spin. > > (and apologies for late reply) > > > A few minor things. > > > > > > 1. I observed larger errors on idle cpus than busy cpus, though it's just up > > > to 1%. > > > Not sure if this comes from the uncertain time interval between the last > > > tick and entering idle. > > > The shorter averaging interval, the larger error, I supposed. > > All right - will look into it. > > Just for my benefit: that diff is strictly between arch_freq_avg_get_on_cpu > > and cpufreq_driver->get(policy->cpu) ? > > I can't say whether it's "strictly" between them or not because driver->get() > shows a fluctuating value. > On my platform, cppc_cpufreq's driver->get() sometimes shows large errors on > busy cpus (as reported by recent emails), but quite accurate on idle cpus (<1%). > > With this patch, the "error" on idle cpus as mentioned above is typically <1%, > hence better in general. Ah, that's great then - I must have misunderstood your previous comment. Apologies for that. > > > > 2. In the current implementation, the resolution of frequency would be: > > > max_freq_khz / SCHED_CAPACITY_SCALE > > > This looks a bit unnecessary to me. > > > > > > It's supposed to get a better resolution if we can do this in > > > arch_freq_get_on_cpu(): > > > > > > freq = delta_cycle_cnt * max_freq_khz / delta_const_cnt > > > > > > which may require caching both current and previous sets of counts in the > > > per-cpu struct amu_cntr_sample. > > > > > arch_freq_get_on_cpu relies on the frequency scale factor to derive the average > > frequency. The scale factor is being calculated based on the deltas you have > > mentioned and arch_max_freq_scale which uses SCHED_CAPACITY_SCALE*2 factor to > > accommodate for rather low reference frequencies. arch_freq_get_on_cpu just does > > somewhat reverse computation to that. > > Yeah I understood this. > > arch_freq_get_on_cpu() now returns: > freq = (arch_scale_freq_capacity() * arch_scale_freq_ref()) >> SCHED_CAPACITY_SHIFT > > The frequency resolution is (arch_scale_freq_ref() >> SCHED_CAPACITY_SHIFT), which > is equivalent to max_freq_khz / SCHED_CAPACITY_SCALE. > > If we can directly do > freq = delta_cycle_cnt * ref_freq_khz / delta_const_cnt > in arch_freq_get_on_cpu(), it's supposed to have a 1KHz resolution. > (sorry for the wrong multiplier in the last reply) > > Just similar to what's done in [1]. > > It could be more worthwhile to have a good resolution than utilising the > arch_topology framework? I guess the question would be whether the precision uplift justifies the change ? In both cases, provided frequency value will carry over potential error due to the nature of how it is being obtained. Furthermore, it is still an average frequency so I am not really convinced that trading here for a fraction of precision would bring any real value. Note that, the difference between the two will fluctuate as well. If there is indeed a real need for getting that extra precision* it would be good to see some actual numbers ? --- BR Beata > > [1]https://lore.kernel.org/all/20240229162520.970986-2-vanshikonda@os.amperecomputing.com/ > > > > > --- > > BR > > Beata > > > Kind regards, > > > Jie > > >