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=-5.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 AA1E5C4361B for ; Thu, 10 Dec 2020 10:39:34 +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 6EAFD23D6A for ; Thu, 10 Dec 2020 10:39:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6EAFD23D6A Authentication-Results: mail.kernel.org; dmarc=fail (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=oRXgfTtO66CNjAJXrfzMHeNb80bpnOzUxlSiJ1ls5kQ=; b=bq700QMNFho/f0Txjw0l+rOeD 3qFpXj8pXyGGsJ7QUxOMkzZZO63lLV0rrMtRYrh3cBmp1yeN0bvtNJiCqu3kmXNaWv9MVApE5XHD1 0eUXBKuuVoZ1KAkJn3K08OkRl/DjnbbxTDJ3hN5yFHgSulUO+VR08eZ7sJbjJqz8aoe6YJjV1PU6i C3J2GpPknPPQWxqSvhY/i7olgdf+kwcdbpSMOkexEtLxioPDvPgLyXcBelPLTVUWLZ8S1GosExqsW Shpfl0CDnha+z0pwTEpGRV4Q/IsauM6bcym8mh3tWoYY9WmnqVRGSumr3T8vkfwwRhf7drnskjdZl si1ROgMFw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1knJL2-0000qf-At; Thu, 10 Dec 2020 10:38:20 +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 1knJKz-0000pk-Hu for linux-arm-kernel@lists.infradead.org; Thu, 10 Dec 2020 10:38:18 +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 E11AC30E; Thu, 10 Dec 2020 02:38:16 -0800 (PST) Received: from localhost (unknown [10.1.198.32]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 833903F718; Thu, 10 Dec 2020 02:38:16 -0800 (PST) Date: Thu, 10 Dec 2020 10:38:15 +0000 From: Ionela Voinescu To: Viresh Kumar Subject: Re: [PATCH] arm64: topology: Cleanup init_amu_fie() a bit Message-ID: <20201210103815.GA3313@arm.com> References: <5594c7d6756a47b473ceb6f48cc217458db32ab0.1607584435.git.viresh.kumar@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5594c7d6756a47b473ceb6f48cc217458db32ab0.1607584435.git.viresh.kumar@linaro.org> 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-20201210_053817_670033_E35831B6 X-CRM114-Status: GOOD ( 17.54 ) 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: Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Vincent Guittot 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, On Thursday 10 Dec 2020 at 12:48:20 (+0530), Viresh Kumar wrote: > Every time I have stumbled upon this routine, I get confused with the > way 'have_policy' is used and I have to dig in to understand why is it > so. > I'm first of all replying to discuss the need of this policy analysis that enable_policy_freq_counters() does which results in the setting of have_policy. Basically, that's functions purpose is only to make sure that invariance at the level of the policy is consistent: either all CPUs in a policy support counters and counters will be used for the scale factor, or either none or only some support counters and therefore the default cpufreq implementation will be used (arch_set_freq_scale()) for all CPUs in a policy. If we find that cpufreq policies are not present at all, we only enable counter use if all CPUs support them. This being said, there is a very important part of your patches in this set: + /* Disallow partial use of counters for frequency invariance */ + if (!cpumask_equal(amu_fie_cpus, cpu_present_mask)) + goto free_valid_mask; If this is agreed upon, there is a lot more that can be removed from the initialisation: enable_policy_freq_counters() can entirely go away plus all the checks around it. I completely agree that all of this will be more clear if we decided to "Disallow partial use of counters for frequency invariance", but I think we might have to add it back in again when systems with partial support for counters show up. I don't agree to not support these systems from the get go as it's likely that the first big.LITTLE systems will only have partial support for AMUs, but it's only an assumption at this point. I'm happy to hear the opinion of other arm64 folk about this. Many thanks for looking over the code, Ionela. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel