Re: [PATCH 1/2] drm/panthor: Add tracepoint for hardware utilisation changes

["Marcin Ślusarz" <marcin.slusarz@arm.com>]

On Tue, Dec 09, 2025 at 04:22:15PM +0000, Karunika Choo wrote:
> On 09/12/2025 13:01, Nicolas Frattaroli wrote:
> > On Monday, 8 December 2025 18:14:53 Central European Standard Time Karunika Choo wrote:
> >> On 03/12/2025 13:56, Nicolas Frattaroli wrote:
> >>> [... snip ...]
> >>> diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> >>> new file mode 100644
> >>> index 000000000000..01013f81e68a
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> >>> @@ -0,0 +1,38 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 or MIT */
> >>> +/* Copyright 2025 Collabora ltd. */
> >>> +
> >>> +#undef TRACE_SYSTEM
> >>> +#define TRACE_SYSTEM panthor
> >>> +
> >>> +#if !defined(__PANTHOR_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
> >>> +#define __PANTHOR_TRACE_H__
> >>> +
> >>> +#include <linux/tracepoint.h>
> >>> +#include <linux/types.h>
> >>> +
> >>> +TRACE_EVENT(gpu_power_active,
> >>> +	TP_PROTO(u64 shader_bitmap, u64 tiler_bitmap, u64 l2_bitmap),
> >>
> >> nit: if you want to add tracing can we also add the device name as
> >> well? Something like:
> >>
> >>   TP_PROTO(struct device *dev, ...),
> >>   TP_ARGS(dev, ...),
> >>   TP_STRUCT__ENTRY(
> >>           __string(dev_name, dev_name(dev))
> >>           ...
> >>   ).
> >>   ...
> > 
> > This is a great idea, will do. Any specific reason to pass the
> > device in the tracepoint rather than a const char*?
> > 
> 
> Nope, totaly fine to do it that way as well.

If you store any pointer into the trace buffer, then by the time
it will be dereferenced (TP_printk below), the object behind it may
be already freed. __string/__assign_str prevents this use-after-free
by embedding a full copy of the string in the trace buffer.

Cheers,
Marcin