linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [for-next][PATCH] tracing: Add __field_struct macro for TRACE_EVENT()
@ 2014-06-21 11:50 Steven Rostedt
  2014-06-21 12:07 ` Borislav Petkov
  0 siblings, 1 reply; 2+ messages in thread
From: Steven Rostedt @ 2014-06-21 11:50 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Andrew Morton, Tony Luck, Borislav Petkov


Updates to the RAS tracepoints found that the __field() macro has a bug
in it where you can not use it with structures. It only works with
primitives. This is because of an added check to determine if the field
is signed or not.

A new macro is created called __field_struct() that can be used to save
structured data directly in the tracepoint and not have to settle for
storing as an array with memcpy.

-- Steve



  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next

Head SHA1: 4d4c9cc839a308be3289a361ccba4447ee140552


Steven Rostedt (1):
      tracing: Add __field_struct macro for TRACE_EVENT()

----
 include/trace/ftrace.h                     | 33 ++++++++++++++++++++++++++++++
 samples/trace_events/trace-events-sample.h |  3 ++-
 2 files changed, 35 insertions(+), 1 deletion(-)
---------------------------
commit 4d4c9cc839a308be3289a361ccba4447ee140552
Author: Steven Rostedt <rostedt@goodmis.org>
Date:   Tue Jun 17 08:59:16 2014 -0400

    tracing: Add __field_struct macro for TRACE_EVENT()
    
    Currently the __field() macro in TRACE_EVENT is only good for primitive
    values, such as integers and pointers, but it fails on complex data types
    such as structures or unions. This is because the __field() macro
    determines if the variable is signed or not with the test of:
    
      (((type)(-1)) < (type)1)
    
    Unfortunately, that fails when type is a structure.
    
    Since trace events should support structures as fields a new macro
    is created for such a case called __field_struct() which acts exactly
    the same as __field() does but it does not do the signed type check
    and just uses a constant false for that answer.
    
    Cc: Tony Luck <tony.luck@gmail.com>
    Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 0fd06fef9fac..26b4f2e13275 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -44,6 +44,12 @@
 #undef __field_ext
 #define __field_ext(type, item, filter_type)	type	item;
 
+#undef __field_struct
+#define __field_struct(type, item)	type	item;
+
+#undef __field_struct_ext
+#define __field_struct_ext(type, item, filter_type)	type	item;
+
 #undef __array
 #define __array(type, item, len)	type	item[len];
 
@@ -122,6 +128,12 @@
 #undef __field_ext
 #define __field_ext(type, item, filter_type)
 
+#undef __field_struct
+#define __field_struct(type, item)
+
+#undef __field_struct_ext
+#define __field_struct_ext(type, item, filter_type)
+
 #undef __array
 #define __array(type, item, len)
 
@@ -315,9 +327,21 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = {	\
 	if (ret)							\
 		return ret;
 
+#undef __field_struct_ext
+#define __field_struct_ext(type, item, filter_type)			\
+	ret = trace_define_field(event_call, #type, #item,		\
+				 offsetof(typeof(field), item),		\
+				 sizeof(field.item),			\
+				 0, filter_type);			\
+	if (ret)							\
+		return ret;
+
 #undef __field
 #define __field(type, item)	__field_ext(type, item, FILTER_OTHER)
 
+#undef __field_struct
+#define __field_struct(type, item) __field_struct_ext(type, item, FILTER_OTHER)
+
 #undef __array
 #define __array(type, item, len)					\
 	do {								\
@@ -379,6 +403,12 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call)	\
 #undef __field_ext
 #define __field_ext(type, item, filter_type)
 
+#undef __field_struct
+#define __field_struct(type, item)
+
+#undef __field_struct_ext
+#define __field_struct_ext(type, item, filter_type)
+
 #undef __array
 #define __array(type, item, len)
 
@@ -550,6 +580,9 @@ static inline notrace int ftrace_get_offsets_##call(			\
 #undef __field
 #define __field(type, item)
 
+#undef __field_struct
+#define __field_struct(type, item)
+
 #undef __array
 #define __array(type, item, len)
 
diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 6af373236d73..4b0113f73ee9 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -56,7 +56,8 @@
  * struct:  This defines the way the data will be stored in the ring buffer.
  *    There are currently two types of elements. __field and __array.
  *    a __field is broken up into (type, name). Where type can be any
- *    type but an array.
+ *    primitive type (integer, long or pointer). __field_struct() can
+ *    be any static complex data value (struct, union, but not an array).
  *    For an array. there are three fields. (type, name, size). The
  *    type of elements in the array, the name of the field and the size
  *    of the array.

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [for-next][PATCH] tracing: Add __field_struct macro for TRACE_EVENT()
  2014-06-21 11:50 [for-next][PATCH] tracing: Add __field_struct macro for TRACE_EVENT() Steven Rostedt
@ 2014-06-21 12:07 ` Borislav Petkov
  0 siblings, 0 replies; 2+ messages in thread
From: Borislav Petkov @ 2014-06-21 12:07 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: LKML, Ingo Molnar, Andrew Morton, Tony Luck

On Sat, Jun 21, 2014 at 07:50:25AM -0400, Steven Rostedt wrote:
> 
> Updates to the RAS tracepoints found that the __field() macro has a bug
> in it where you can not use it with structures. It only works with
> primitives. This is because of an added check to determine if the field
> is signed or not.
> 
> A new macro is created called __field_struct() that can be used to save
> structured data directly in the tracepoint and not have to settle for
> storing as an array with memcpy.
> 
> -- Steve
> 
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> for-next
> 
> Head SHA1: 4d4c9cc839a308be3289a361ccba4447ee140552
> 
> 
> Steven Rostedt (1):
>       tracing: Add __field_struct macro for TRACE_EVENT()
> 
> ----
>  include/trace/ftrace.h                     | 33 ++++++++++++++++++++++++++++++
>  samples/trace_events/trace-events-sample.h |  3 ++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> ---------------------------
> commit 4d4c9cc839a308be3289a361ccba4447ee140552
> Author: Steven Rostedt <rostedt@goodmis.org>
> Date:   Tue Jun 17 08:59:16 2014 -0400
> 
>     tracing: Add __field_struct macro for TRACE_EVENT()
>     
>     Currently the __field() macro in TRACE_EVENT is only good for primitive
>     values, such as integers and pointers, but it fails on complex data types
>     such as structures or unions. This is because the __field() macro
>     determines if the variable is signed or not with the test of:
>     
>       (((type)(-1)) < (type)1)
>     
>     Unfortunately, that fails when type is a structure.
>     
>     Since trace events should support structures as fields a new macro
>     is created for such a case called __field_struct() which acts exactly
>     the same as __field() does but it does not do the signed type check
>     and just uses a constant false for that answer.
>     
>     Cc: Tony Luck <tony.luck@gmail.com>
>     Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-06-21 12:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-21 11:50 [for-next][PATCH] tracing: Add __field_struct macro for TRACE_EVENT() Steven Rostedt
2014-06-21 12:07 ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).