[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <28868c0e-2a46-47ed-9bd7-439056cf94c0@arm.com>
Date: Mon, 17 Nov 2025 09:18:35 +0000
From: Christian Loehle <christian.loehle@....com>
To: Viresh Kumar <viresh.kumar@...aro.org>, Samuel Wu <wusamuel@...gle.com>,
peterz@...radead.org, vincent.guittot@...aro.org
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
<mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
kernel-team@...roid.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
Dietmar Eggemann <dietmar.eggemann@....com>,
Qais Yousef <qyousef@...alina.io>, John Stultz <jstultz@...gle.com>
Subject: Re: [PATCH v1] cpufreq: Add policy_frequency trace event
On 11/14/25 05:11, Viresh Kumar wrote:
> On 13-11-25, 19:41, Samuel Wu wrote:
>> On Wed, Nov 12, 2025 at 10:45 PM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>>>
>>> On 12-11-25, 15:51, Samuel Wu wrote:
>>>> The existing cpu_frequency trace_event can be verbose, emitting an event
>>>> for every CPU in the policy even when their frequencies are identical.
>>>>
>>>> This patch adds a new policy_frequency trace event, which provides a
>>>> more efficient alternative to cpu_frequency trace event. This option
>>>> allows users who only need frequency at a policy level more concise logs
>>>> with simpler analysis.
>>>>
>>>> Signed-off-by: Samuel Wu <wusamuel@...gle.com>
>>>> ---
>>>> drivers/cpufreq/cpufreq.c | 2 ++
>>>> include/trace/events/power.h | 21 +++++++++++++++++++++
>>>> 2 files changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>>> index 4472bb1ec83c..b65534a4fd9a 100644
>>>> --- a/drivers/cpufreq/cpufreq.c
>>>> +++ b/drivers/cpufreq/cpufreq.c
>>>> @@ -345,6 +345,7 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
>>>> pr_debug("FREQ: %u - CPUs: %*pbl\n", freqs->new,
>>>> cpumask_pr_args(policy->cpus));
>>>>
>>>> + trace_policy_frequency(freqs->new, policy->cpu);
>>>> for_each_cpu(cpu, policy->cpus)
>>>> trace_cpu_frequency(freqs->new, cpu);
>>>
>>> I don't see much value in almost duplicate trace events. If we feel that a
>>> per-policy event is a better fit (which makes sens), then we can just drop the
>>> trace_cpu_frequency() events and print policy->cpus (or related_cpus)
>>> information along with the per-policy events.
>>
>> Thank you for the feedback Viresh. Fair enough, I've done some testing
>> and a single trace event should work and would be cleaner. Please let
>> me know what you think of this proposal for v2.
>>
>> We can append a bitmask of policy->cpus field to
>> trace_cpu_frequency(). This way we maintain backwards compatibility:
>> trace_cpu_frequency() is not removed, and its pre-existing fields are
>> not disturbed.
>>
>> Call flow wise, we can delete all the for_each_cpu() loops, and we
>> still retain the benefits of the trace emitting once per policy
>> instead of once per cpu.
>
> Fine by me. I have added Scheduler maintainers in the loop to see if they have a
> different view.
>
And IIUC your proposal is to fold policy_frequency into cpu_frequency but then
only have one cpu_frequency event per policy emitted?
I think from a tooling perspective it would be easier to remove cpu_frequency
entirely, then tools can probe on the presence of policy_frequency / cpu_frequency.
Powered by blists - more mailing lists