On Wed, Jul 10, 2019 at 08:12:30PM +0300, Ville Syrjala wrote:
> From: "Steven Rostedt (VMware)" <rostedt@xxxxxxxxxxx>
>
> Currently the intel_update_plane and intel_disable_plane tracepoints record
> the address of plane->name in the ring buffer, and then when reading the
> ring buffer uses %s to get the name. The issue with this, is that those two
> events can be minutes, hours or even days apart. It is very dangerous to
> dereference a string pointer without knowing if it still exists or not.
>
> The proper way to handle this is to use the __string() macro in the
> tracepoint which will save the string into the ring buffer at the time of
> recording. Then there's no worries if the original string still exists in
> memory when the ring buffer is read.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> [vsyrjala: Rebase on top of drm-tip]
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
CI is happy (not that we test this stuff) and I'm happy (the tracepoints
still work) -> pushed to drm-intel-next-queued. Thanks for the patch.
> ---
> drivers/gpu/drm/i915/i915_trace.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index cce426b23a24..da18b8d6b80c 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -293,16 +293,16 @@ TRACE_EVENT(intel_update_plane,
>
> TP_STRUCT__entry(
> __field(enum pipe, pipe)
> - __field(const char *, name)
> __field(u32, frame)
> __field(u32, scanline)
> __array(int, src, 4)
> __array(int, dst, 4)
> + __string(name, plane->name)
> ),
>
> TP_fast_assign(
> + __assign_str(name, plane->name);
> __entry->pipe = crtc->pipe;
> - __entry->name = plane->name;
> __entry->frame = intel_crtc_get_vblank_counter(crtc);
> __entry->scanline = intel_get_crtc_scanline(crtc);
> memcpy(__entry->src, &plane->state->src, sizeof(__entry->src));
> @@ -310,7 +310,7 @@ TRACE_EVENT(intel_update_plane,
> ),
>
> TP_printk("pipe %c, plane %s, frame=%u, scanline=%u, " DRM_RECT_FP_FMT " -> " DRM_RECT_FMT,
> - pipe_name(__entry->pipe), __entry->name,
> + pipe_name(__entry->pipe), __get_str(name),
> __entry->frame, __entry->scanline,
> DRM_RECT_FP_ARG((const struct drm_rect *)__entry->src),
> DRM_RECT_ARG((const struct drm_rect *)__entry->dst))
> @@ -322,20 +322,20 @@ TRACE_EVENT(intel_disable_plane,
>
> TP_STRUCT__entry(
> __field(enum pipe, pipe)
> - __field(const char *, name)
> __field(u32, frame)
> __field(u32, scanline)
> + __string(name, plane->name)
> ),
>
> TP_fast_assign(
> + __assign_str(name, plane->name);
> __entry->pipe = crtc->pipe;
> - __entry->name = plane->name;
> __entry->frame = intel_crtc_get_vblank_counter(crtc);
> __entry->scanline = intel_get_crtc_scanline(crtc);
> ),
>
> TP_printk("pipe %c, plane %s, frame=%u, scanline=%u",
> - pipe_name(__entry->pipe), __entry->name,
> + pipe_name(__entry->pipe), __get_str(name),
> __entry->frame, __entry->scanline)
> );
>
> --
> 2.21.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
