Messages in this thread | | | Date | Mon, 27 Feb 2023 10:07:15 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v3 20/20] thermal/traces: Replace the thermal zone structure parameter with the field value |
| |
On Sun, 26 Feb 2023 23:54:06 +0100 Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> In the work of the thermal zone device self-encapsulation, let's pass > the field value instead of dereferencing them in the traces which > force us to export publicly the thermal_zone_device structure. > > No fonctionnal change intended. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/gov_fair_share.c | 4 +++- > drivers/thermal/gov_power_allocator.c | 6 +++-- > drivers/thermal/gov_step_wise.c | 4 +++- > drivers/thermal/thermal_core.c | 8 +++++-- > include/trace/events/thermal.h | 24 +++++++++---------- > .../trace/events/thermal_power_allocator.h | 12 +++++----- > 6 files changed, 34 insertions(+), 24 deletions(-) > > diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c > index aad7d5fe3a14..cdddd593021d 100644 > --- a/drivers/thermal/gov_fair_share.c > +++ b/drivers/thermal/gov_fair_share.c > @@ -35,7 +35,9 @@ static int get_trip_level(struct thermal_zone_device *tz) > * point, in which case, trip_point = count - 1 > */ > if (count > 0) > - trace_thermal_zone_trip(tz, count - 1, trip.type); > + trace_thermal_zone_trip(thermal_zone_device_type(tz), > + thermal_zone_device_id(tz), > + count - 1, trip.type);
The problem with this approach is that you are moving all the work to dereference the pointers into the hot paths (the code execution), instead of doing it in the slow path (where the tracepoint work is done).
If you are concerned with exporting a structure then move the trace file from:
include/trace/events/thermal.h to drivers/thermal/trace.h
like drivers/vfio/pci/trace.h and many other drivers do.
Read https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/trace_events/Makefile to see how to use a trace header outside the include/trace/events directory.
also, by removing the pointer, you lose out on BPF and kprobe traces that could dereference the pointer if you needed to trace something that was not exported by the trace point itself.
-- Steve
> > return count; > }
| |