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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9164C4167B for ; Fri, 9 Dec 2022 08:39:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229776AbiLIIjU (ORCPT ); Fri, 9 Dec 2022 03:39:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46638 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229521AbiLIIjS (ORCPT ); Fri, 9 Dec 2022 03:39:18 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id E84AD4D5C6; Fri, 9 Dec 2022 00:39:16 -0800 (PST) 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 1409323A; Fri, 9 Dec 2022 00:39:23 -0800 (PST) Received: from [10.57.9.37] (unknown [10.57.9.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9D92A3F73B; Fri, 9 Dec 2022 00:39:13 -0800 (PST) Message-ID: Date: Fri, 9 Dec 2022 08:39:10 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v3 1/1] cpufreq: schedutil: Optimize operations with single CPU capacity lookup Content-Language: en-US To: Viresh Kumar Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, dietmar.eggemann@arm.com, vincent.guittot@linaro.org, saravanak@google.com, wusamuel@google.com, isaacmanjarres@google.com, kernel-team@android.com, juri.lelli@redhat.com, peterz@infradead.org, mingo@redhat.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de References: <20221208160256.859-1-lukasz.luba@arm.com> <20221208160256.859-2-lukasz.luba@arm.com> <20221208233801.s26awslkx6aloxyd@vireshk-i7> From: Lukasz Luba In-Reply-To: <20221208233801.s26awslkx6aloxyd@vireshk-i7> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/8/22 23:38, Viresh Kumar wrote: > On 08-12-22, 16:02, Lukasz Luba wrote: >> @@ -332,12 +333,15 @@ static void sugov_update_single_freq(struct update_util_data *hook, u64 time, >> struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_util); >> struct sugov_policy *sg_policy = sg_cpu->sg_policy; >> unsigned int cached_freq = sg_policy->cached_raw_freq; >> + unsigned long max_cap; >> unsigned int next_f; >> >> - if (!sugov_update_single_common(sg_cpu, time, flags)) >> + max_cap = arch_scale_cpu_capacity(sg_cpu->cpu); > > I will rather do this at all three locations: > > unsigned long max_cap = arch_scale_cpu_capacity(sg_cpu->cpu); > In the 2nd location it is called after the check: if (!arch_scale_freq_invariant()) which can return. IMO this is more visible and exposed. The way it's implemented now stresses the fact that we read this value at runtime (unfortunately). Maybe in the future someone would find it and simplify. I sometimes found difficult to spot those important calls in the variable header section, e.g. how many times are called or with what kind of arguments. In this case the sg_cpu->cpu should be clearly visible and effectively matched as the same as smp_procesor_id() for that running CPU, thus fetching the same capacity variable from local per-cpu (not remote). IMO the way how the code is structured could help (or not) to spot those details. That's why I prefer to keep it as is in this implementation.