Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9994acf8-e0bc-55ce-9012-e36ef3b8ddab@arm.com>
Date:   Tue, 11 Apr 2023 17:39:23 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 05/17] trace: energy_model: Add trace event for EM runtime
 modifications
Content-Language: en-US
To:     Lukasz Luba <lukasz.luba@arm.com>, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, rafael@kernel.org
Cc:     dietmar.eggemann@arm.com, rui.zhang@intel.com,
        amit.kucheria@verdurent.com, amit.kachhap@gmail.com,
        daniel.lezcano@linaro.org, viresh.kumar@linaro.org,
        len.brown@intel.com, pavel@ucw.cz, ionela.voinescu@arm.com,
        rostedt@goodmis.org, mhiramat@kernel.org
References: <20230314103357.26010-1-lukasz.luba@arm.com>
 <20230314103357.26010-6-lukasz.luba@arm.com>
From:   Pierre Gondois <pierre.gondois@arm.com>
In-Reply-To: <20230314103357.26010-6-lukasz.luba@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello Lukasz,
Just a suggestion, maybe it would be good to trace the CPUs affected by
the modification. It is possible to retrieve this information by going
to /sys/kernel/debug/energy_model/xxx/cpus, but might be simpler when
parsing a trace.

Regards,
Pierre

On 3/14/23 11:33, Lukasz Luba wrote:
> The Energy Model (EM) supports runtime modifications. Track the changes
> in order to do post-processing analysis. Don't use arrays in the trace
> event, since they are not properly supported by the tools. Instead use
> simple "unroll" with emitting the trace event for each EM array entry
> with proper ID information. The older debugging mechanism which was
> the simple debugfs which dumping the EM content won't be sufficient for
> the modifiable EM purpose. This trace event mechanism would address the
> needs.
> 
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>   include/trace/events/energy_model.h | 46 +++++++++++++++++++++++++++++
>   kernel/power/energy_model.c         |  3 ++
>   2 files changed, 49 insertions(+)
>   create mode 100644 include/trace/events/energy_model.h
> 
> diff --git a/include/trace/events/energy_model.h b/include/trace/events/energy_model.h
> new file mode 100644
> index 000000000000..f70babeb5dde
> --- /dev/null
> +++ b/include/trace/events/energy_model.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM energy_model
> +
> +#if !defined(_TRACE_ENERGY_MODEL_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ENERGY_MODEL_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(em_perf_state,
> +	TP_PROTO(const char *dev_name, int nr_perf_states, int state,
> +		 unsigned long ps_frequency, unsigned long ps_power,
> +		 unsigned long ps_cost, unsigned long ps_flags),
> +
> +	TP_ARGS(dev_name, nr_perf_states, state, ps_frequency, ps_power, ps_cost,
> +		ps_flags),
> +
> +	TP_STRUCT__entry(
> +		__string(name, dev_name)
> +		__field(int, num_states)
> +		__field(int, state)
> +		__field(unsigned long, frequency)
> +		__field(unsigned long, power)
> +		__field(unsigned long, cost)
> +		__field(unsigned long, flags)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(name, dev_name);
> +		__entry->num_states = nr_perf_states;
> +		__entry->state = state;
> +		__entry->frequency = ps_frequency;
> +		__entry->power = ps_power;
> +		__entry->cost = ps_cost;
> +		__entry->flags = ps_flags;
> +	),
> +
> +	TP_printk("dev_name=%s nr_perf_states=%d state=%d frequency=%lu power=%lu cost=%lu flags=%lu",
> +		__get_str(name), __entry->num_states, __entry->state,
> +		__entry->frequency, __entry->power, __entry->cost,
> +		__entry->flags)
> +);
> +#endif /* _TRACE_ENERGY_MODEL_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index 937e98a71ed5..3b778743ba89 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -17,6 +17,9 @@
>   #include <linux/sched/topology.h>
>   #include <linux/slab.h>
>   
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/energy_model.h>
> +
>   /*
>    * Mutex serializing the registrations of performance domains and letting
>    * callbacks defined by drivers sleep.