Re: [PATCH v3 3/3] trace: thermal: add another parameter *power to the tracing function

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 14 Mar 2017 13:06:16 +0000
Lukasz Luba <lukasz.luba@arm.com> wrote:

> This patch adds another parameter to the trace function:
> trace_thermal_power_devfreq_get_power().
> 
> In case when we call directly driver's code for the real power,
> we do not have static/dynamic_power values. Instead we get total
> power in the '*power' value. The 'static_power' and
> 'dynamic_power' are set to 0.
> 
> Therefore, we have to trace that '*power' value in this scenario.
> 
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Zhang Rui <rui.zhang@intel.com>
> CC: Eduardo Valentin <edubezval@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/thermal/devfreq_cooling.c |  2 +-
>  include/trace/events/thermal.h    | 11 +++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index 4411ab8..ca3ebe3 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -304,7 +304,7 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
>  	}
>  
>  	trace_thermal_power_devfreq_get_power(cdev, status, freq, dyn_power,
> -					      static_power);
> +					      static_power, *power);

I'm curious. Can you disassemble the above, and take a look at how it
handles that dereference?

I may make sense to pass in the pointer and do the dereference in the
TP_fast_assign():

	__entry->power = *power;

I like to make the call site as light as possible and do as much work
in the tracepoint code that is feasible. This helps keep tracepoints
light weight when tracing is off.

-- Steve


>  
>  	return 0;