Re: [patch 014/173] mm, tracing: record slab name for kmem_cache_free()

On 2/25/21 10:07 AM, Steven Rostedt wrote:

On Wed, 24 Feb 2021 20:37:08 -0500
Steven Rostedt [off-list ref] wrote:

quoted

quoted

+	TP_printk("call_site=%pS ptr=%p name=%s",
+		  (void *)__entry->call_site, __entry->ptr, __entry->name)

	You must use __get_str(name) here!

(see other users of this logic in include/trace/events/*.h)

What is happening is that TP_fast_assign() is called by the tracepoint
logic (at the time of the event), then much later (seconds, minutes,
hours, days!), when the user does a "cat trace" of the file, the
__entry->name is read and the printf logic is called against it. Well,
the s->name that __entry->name points to, can be long gone by then!

Instead, using __string() tells the TRACE_EVENT() macro that this is a
dynamic string. The __assign_str() records the string into the ring
buffer. The __get_str() retrieves the string from the ring buffer as
part of the event, so it stays around as long as the event being read
by the trace file is around.

Please do not apply this patch as is, it is very buggy!

I wonder if we can add something to checkpatch that can check if
TP_printk() has a call to "%s" where it references a __entry->xxx and
not a __get_str(), and will warn about it.

That's helpful for me who don't know "%s" of TP_printk is special.

There a a few cases where its OK. Like RCU uses a TPS() macro around
strings it passes into the tracepoint, which is used for strings that
never are freed, and maps the string pointer to the string for user
space. But RCU is the only user of that I believe.

-- Steve