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.7 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 E9BCEC4363D for ; Tue, 22 Sep 2020 16:08:48 +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 6F1E822206 for ; Tue, 22 Sep 2020 16:08:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Z2XDt96C" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6F1E822206 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=qdfZwHk31RAXJlDJub4oKGK/9Rf0kfcqCJSMNkRJOHg=; b=Z2XDt96CH15S5n9DYNTMYDh4G zw5cLeFMA1loqK5DOn7CAT0oMmNVW7jsTgm6Lij/Ez9FkSEXWe2C2t6blvVptlnXTv0KNzcWBJLsb f0t6OqMXc/uWWW7n5rmVDNsTmCrhkgY3ICaruAQ8pKl/MUJlurYTqbf9YNBea6J5YOcaaUuFwoHWS wjT9tQxnRA2GMiZ+7dJkdLCb05exhqrUDjonjluufZhBripbr6QrEkOq1G08cZe+sgQGifgg7AZBP Rzq4w7uJ0CdW2IiJdQohN1ltoPjR2moYTiK58rLQ6Mtt6yT4djRFQNZQVtiFvP/6xHGqb4UgjtRf/ uU8D8fWZA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kKkoz-0000kw-1V; Tue, 22 Sep 2020 16:07:13 +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 1kKkov-0000jo-1t for linux-arm-kernel@lists.infradead.org; Tue, 22 Sep 2020 16:07:10 +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 4B210101E; Tue, 22 Sep 2020 09:07:06 -0700 (PDT) Received: from localhost (e108754-lin.cambridge.arm.com [10.1.199.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E04923F718; Tue, 22 Sep 2020 09:07:05 -0700 (PDT) Date: Tue, 22 Sep 2020 17:07:04 +0100 From: Ionela Voinescu To: Catalin Marinas Subject: Re: [PATCH 4/4] arm64: implement CPPC FFH support using AMUs Message-ID: <20200922160704.GA796@arm.com> References: <20200826130309.28027-1-ionela.voinescu@arm.com> <20200826130309.28027-5-ionela.voinescu@arm.com> <20200911144032.GC12835@gaia> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200911144032.GC12835@gaia> 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-20200922_120709_247103_8A94BEAF X-CRM114-Status: GOOD ( 24.55 ) 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: souvik.chakravarty@arm.com, viresh.kumar@linaro.org, linux-kernel@vger.kernel.org, morten.rasmussen@arm.com, linux-arm-kernel@lists.infradead.org, sudeep.holla@arm.com, will@kernel.org, valentin.schneider@arm.com, dietmar.eggemann@arm.com 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 Catalin, Sorry for the delayed reply. I took advantage of a last chance for a holiday before the weather gets bad :). On Friday 11 Sep 2020 at 15:40:32 (+0100), Catalin Marinas wrote: > On Wed, Aug 26, 2020 at 02:03:09PM +0100, Ionela Voinescu wrote: > > +/* > > + * Refer to drivers/acpi/cppc_acpi.c for the description of the functions > > + * below. > > + */ > > +bool cpc_ffh_supported(void) > > +{ > > + const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters(); > > + int cpu = nr_cpu_ids; > > + > > + if (cnt_cpu_mask) > > + cpu = cpumask_any_and(cnt_cpu_mask, cpu_present_mask); > > + > > + if ((cpu >= nr_cpu_ids) || !freq_counters_valid(cpu)) > > + return false; > > + > > + return true; > > +} > > IIUC, the only need for the cpumask is this function, the others would > have worked just fine with the existing cpu_has_amu_feat(). So you have > a lot more !cnt_cpu_mask checks now. > > I wonder whether instead you could add a new function near > cpu_has_amu_feat(), something like get_cpu_with_amu_feat() and do the > cpumask_any_and() in there. > Yes, it does look ugly. I went for this because I wanted to avoid adding new functions to the cpu feature code with new AMU usecase additions, functions that might just end up doing cpumask operations, and nothing more. This way there is a single function that basically extracts all the information that the feature code is able to provide on AMU support and the user of the counters can then manipulate the mask as it sees fit. Basically I was thinking that with a potential new usecase we might have to add yet another function and I did not like that prospect. >From performance point of view, the !cnt_cpu_mask checks wouldn't affect that much: cpc_ffh_supported() is only called during CPPC probe and freq_counters_valid() is only called at init. counters_read_on_cpu() will be called more often (cpufreq .get function) but not as much as to bring significant overhead. To make this nicer I can do the following: - make the !cnt_cpu_mask unlikely due to CONFIG_ARM64_AMU_EXTN being enabled by default. - wrap all the cpus_with_amu_counters() uses under: static inline int amu_cpus_any_and(const struct cpumask *cpus) { const struct cpumask *cnt_cpu_mask = cpus_with_amu_counters(); int cpu = nr_cpu_ids; if (likely(cnt_cpu_mask)) cpu = cpumask_any_and(cnt_cpu_mask, cpus); return cpu; } This is to be called as: - "if (!freq_counters_valid(amu_cpus_any_and(cpu_present_mask)))" in cpc_ffh_supported(); - "if (amu_cpus_any_and(cpumask_of(cpu)) == cpu)" in the other two. It won't eliminate the useless checks but it will make the code a bit nicer. If you don't think it's worth it, I'll go with your suggestion. Thank you for the review, Ionela. > -- > Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel