* [PATCH] tracing: Fix array size mismatch in format string
@ 2014-02-14 3:51 Vaibhav Nagarnaik
2014-03-13 0:16 ` Vaibhav Nagarnaik
0 siblings, 1 reply; 6+ messages in thread
From: Vaibhav Nagarnaik @ 2014-02-14 3:51 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Laurent Chavey, linux-kernel, Vaibhav Nagarnaik
In event format strings, the array size is reported in two locations.
One in array subscript and then via the "size:" attribute. The values
reported there have a mismatch.
For e.g., in sched:sched_switch the prev_comm and next_comm character
arrays have subscript values as [32] where as the actual field size is
16.
name: sched_switch
ID: 301
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:char prev_comm[32]; offset:8; size:16; signed:1;
field:pid_t prev_pid; offset:24; size:4; signed:1;
field:int prev_prio; offset:28; size:4; signed:1;
field:long prev_state; offset:32; size:8; signed:1;
field:char next_comm[32]; offset:40; size:16; signed:1;
field:pid_t next_pid; offset:56; size:4; signed:1;
field:int next_prio; offset:60; size:4; signed:1;
After bisection, the following commit was blamed:
92edca0 tracing: Use direct field, type and system names
This commit removes the duplication of strings for field->name and
field->type assuming that all the strings passed in
__trace_define_field() are immutable. This is not true for arrays, where
the type string is created in event_storage variable and field->type for
all array fields points to event_storage.
Use __stringify() to create a string constant for the type string.
Also, get rid of event_storage and event_storage_mutex that are not
needed anymore.
Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
---
* Fix warning from previous version about mixed declarations.
include/linux/ftrace_event.h | 4 ----
include/trace/ftrace.h | 7 ++-----
kernel/trace/trace_events.c | 6 ------
kernel/trace/trace_export.c | 7 ++-----
4 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 4e4cc28..4cdb3a1 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -495,10 +495,6 @@ enum {
FILTER_TRACE_FN,
};
-#define EVENT_STORAGE_SIZE 128
-extern struct mutex event_storage_mutex;
-extern char event_storage[EVENT_STORAGE_SIZE];
-
extern int trace_event_raw_init(struct ftrace_event_call *call);
extern int trace_define_field(struct ftrace_event_call *call, const char *type,
const char *name, int offset, int size,
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 1a8b28d..1ee19a2 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -310,15 +310,12 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = { \
#undef __array
#define __array(type, item, len) \
do { \
- mutex_lock(&event_storage_mutex); \
+ char *type_str = #type"["__stringify(len)"]"; \
BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
- snprintf(event_storage, sizeof(event_storage), \
- "%s[%d]", #type, len); \
- ret = trace_define_field(event_call, event_storage, #item, \
+ ret = trace_define_field(event_call, type_str, #item, \
offsetof(typeof(field), item), \
sizeof(field.item), \
is_signed_type(type), FILTER_OTHER); \
- mutex_unlock(&event_storage_mutex); \
if (ret) \
return ret; \
} while (0);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e71ffd4..22826c7 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -27,12 +27,6 @@
DEFINE_MUTEX(event_mutex);
-DEFINE_MUTEX(event_storage_mutex);
-EXPORT_SYMBOL_GPL(event_storage_mutex);
-
-char event_storage[EVENT_STORAGE_SIZE];
-EXPORT_SYMBOL_GPL(event_storage);
-
LIST_HEAD(ftrace_events);
static LIST_HEAD(ftrace_common_fields);
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 7c3e3e7..ee0a509 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -95,15 +95,12 @@ static void __always_unused ____ftrace_check_##name(void) \
#undef __array
#define __array(type, item, len) \
do { \
+ char *type_str = #type"["__stringify(len)"]"; \
BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
- mutex_lock(&event_storage_mutex); \
- snprintf(event_storage, sizeof(event_storage), \
- "%s[%d]", #type, len); \
- ret = trace_define_field(event_call, event_storage, #item, \
+ ret = trace_define_field(event_call, type_str, #item, \
offsetof(typeof(field), item), \
sizeof(field.item), \
is_signed_type(type), filter_type); \
- mutex_unlock(&event_storage_mutex); \
if (ret) \
return ret; \
} while (0);
--
1.9.0.rc1.175.g0b1dcb5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Fix array size mismatch in format string
2014-02-14 3:51 [PATCH] tracing: Fix array size mismatch in format string Vaibhav Nagarnaik
@ 2014-03-13 0:16 ` Vaibhav Nagarnaik
2014-03-13 0:53 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Vaibhav Nagarnaik @ 2014-03-13 0:16 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Laurent Chavey, linux-kernel, Vaibhav Nagarnaik
Hi Steven,
Any chance you can take a look at this patch?
Thanks
Vaibhav
On Thu, Feb 13, 2014 at 7:51 PM, Vaibhav Nagarnaik
<vnagarnaik@google.com> wrote:
> In event format strings, the array size is reported in two locations.
> One in array subscript and then via the "size:" attribute. The values
> reported there have a mismatch.
>
> For e.g., in sched:sched_switch the prev_comm and next_comm character
> arrays have subscript values as [32] where as the actual field size is
> 16.
>
> name: sched_switch
> ID: 301
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> field:unsigned char common_flags; offset:2; size:1; signed:0;
> field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:char prev_comm[32]; offset:8; size:16; signed:1;
> field:pid_t prev_pid; offset:24; size:4; signed:1;
> field:int prev_prio; offset:28; size:4; signed:1;
> field:long prev_state; offset:32; size:8; signed:1;
> field:char next_comm[32]; offset:40; size:16; signed:1;
> field:pid_t next_pid; offset:56; size:4; signed:1;
> field:int next_prio; offset:60; size:4; signed:1;
>
> After bisection, the following commit was blamed:
> 92edca0 tracing: Use direct field, type and system names
>
> This commit removes the duplication of strings for field->name and
> field->type assuming that all the strings passed in
> __trace_define_field() are immutable. This is not true for arrays, where
> the type string is created in event_storage variable and field->type for
> all array fields points to event_storage.
>
> Use __stringify() to create a string constant for the type string.
>
> Also, get rid of event_storage and event_storage_mutex that are not
> needed anymore.
>
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
> ---
> * Fix warning from previous version about mixed declarations.
>
> include/linux/ftrace_event.h | 4 ----
> include/trace/ftrace.h | 7 ++-----
> kernel/trace/trace_events.c | 6 ------
> kernel/trace/trace_export.c | 7 ++-----
> 4 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 4e4cc28..4cdb3a1 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -495,10 +495,6 @@ enum {
> FILTER_TRACE_FN,
> };
>
> -#define EVENT_STORAGE_SIZE 128
> -extern struct mutex event_storage_mutex;
> -extern char event_storage[EVENT_STORAGE_SIZE];
> -
> extern int trace_event_raw_init(struct ftrace_event_call *call);
> extern int trace_define_field(struct ftrace_event_call *call, const char *type,
> const char *name, int offset, int size,
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index 1a8b28d..1ee19a2 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -310,15 +310,12 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = { \
> #undef __array
> #define __array(type, item, len) \
> do { \
> - mutex_lock(&event_storage_mutex); \
> + char *type_str = #type"["__stringify(len)"]"; \
> BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
> - snprintf(event_storage, sizeof(event_storage), \
> - "%s[%d]", #type, len); \
> - ret = trace_define_field(event_call, event_storage, #item, \
> + ret = trace_define_field(event_call, type_str, #item, \
> offsetof(typeof(field), item), \
> sizeof(field.item), \
> is_signed_type(type), FILTER_OTHER); \
> - mutex_unlock(&event_storage_mutex); \
> if (ret) \
> return ret; \
> } while (0);
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index e71ffd4..22826c7 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -27,12 +27,6 @@
>
> DEFINE_MUTEX(event_mutex);
>
> -DEFINE_MUTEX(event_storage_mutex);
> -EXPORT_SYMBOL_GPL(event_storage_mutex);
> -
> -char event_storage[EVENT_STORAGE_SIZE];
> -EXPORT_SYMBOL_GPL(event_storage);
> -
> LIST_HEAD(ftrace_events);
> static LIST_HEAD(ftrace_common_fields);
>
> diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
> index 7c3e3e7..ee0a509 100644
> --- a/kernel/trace/trace_export.c
> +++ b/kernel/trace/trace_export.c
> @@ -95,15 +95,12 @@ static void __always_unused ____ftrace_check_##name(void) \
> #undef __array
> #define __array(type, item, len) \
> do { \
> + char *type_str = #type"["__stringify(len)"]"; \
> BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
> - mutex_lock(&event_storage_mutex); \
> - snprintf(event_storage, sizeof(event_storage), \
> - "%s[%d]", #type, len); \
> - ret = trace_define_field(event_call, event_storage, #item, \
> + ret = trace_define_field(event_call, type_str, #item, \
> offsetof(typeof(field), item), \
> sizeof(field.item), \
> is_signed_type(type), filter_type); \
> - mutex_unlock(&event_storage_mutex); \
> if (ret) \
> return ret; \
> } while (0);
> --
> 1.9.0.rc1.175.g0b1dcb5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Fix array size mismatch in format string
2014-03-13 0:16 ` Vaibhav Nagarnaik
@ 2014-03-13 0:53 ` Steven Rostedt
2014-03-13 1:13 ` Vaibhav Nagarnaik
2014-03-18 19:08 ` Vaibhav Nagarnaik
0 siblings, 2 replies; 6+ messages in thread
From: Steven Rostedt @ 2014-03-13 0:53 UTC (permalink / raw)
To: Vaibhav Nagarnaik; +Cc: Laurent Chavey, linux-kernel
On Wed, 12 Mar 2014 17:16:20 -0700
Vaibhav Nagarnaik <vnagarnaik@google.com> wrote:
> Hi Steven,
>
> Any chance you can take a look at this patch?
>
Grumble, I thought I pulled this in already. I may have been working on
it and then got distracted by my day job, and it got lost in the
shuffle.
Your timing here isn't that great either, because I leave tomorrow for
another conference, and I'm currently trying to get everything ready
for that trip. Could you ping me again on Tuesday?
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Fix array size mismatch in format string
2014-03-13 0:53 ` Steven Rostedt
@ 2014-03-13 1:13 ` Vaibhav Nagarnaik
2014-03-18 19:08 ` Vaibhav Nagarnaik
1 sibling, 0 replies; 6+ messages in thread
From: Vaibhav Nagarnaik @ 2014-03-13 1:13 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Laurent Chavey, linux-kernel
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> Your timing here isn't that great either, because I leave tomorrow for
> another conference, and I'm currently trying to get everything ready
> for that trip. Could you ping me again on Tuesday?
Will do.
Vaibhav
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Fix array size mismatch in format string
2014-03-13 0:53 ` Steven Rostedt
2014-03-13 1:13 ` Vaibhav Nagarnaik
@ 2014-03-18 19:08 ` Vaibhav Nagarnaik
1 sibling, 0 replies; 6+ messages in thread
From: Vaibhav Nagarnaik @ 2014-03-18 19:08 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Laurent Chavey, linux-kernel
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> Your timing here isn't that great either, because I leave tomorrow for
> another conference, and I'm currently trying to get everything ready
> for that trip. Could you ping me again on Tuesday?
Hey Steven
It's your friendly reminder to look at the patch :-).
Vaibhav
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] tracing: Fix array size mismatch in format string
@ 2014-02-13 23:05 Vaibhav Nagarnaik
0 siblings, 0 replies; 6+ messages in thread
From: Vaibhav Nagarnaik @ 2014-02-13 23:05 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Laurent Chavey, linux-kernel, Vaibhav Nagarnaik
In event format strings, the array size is reported in two locations.
One in array subscript and then via the "size:" attribute. The values
reported there have a mismatch.
For e.g., in sched:sched_switch the prev_comm and next_comm character
arrays have subscript values as [32] where as the actual field size is
16.
name: sched_switch
ID: 301
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:char prev_comm[32]; offset:8; size:16; signed:1;
field:pid_t prev_pid; offset:24; size:4; signed:1;
field:int prev_prio; offset:28; size:4; signed:1;
field:long prev_state; offset:32; size:8; signed:1;
field:char next_comm[32]; offset:40; size:16; signed:1;
field:pid_t next_pid; offset:56; size:4; signed:1;
field:int next_prio; offset:60; size:4; signed:1;
After bisection, the following commit was blamed:
92edca0 tracing: Use direct field, type and system names
This commit removes the duplication of strings for field->name and
field->type assuming that all the strings passed in
__trace_define_field() are immutable. This is not true for arrays, where
the type string is created in event_storage variable and field->type for
all array fields points to event_storage.
Use __stringify() to create a string constant for the type string.
Also, get rid of event_storage and event_storage_mutex that are not
needed anymore.
Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
---
include/linux/ftrace_event.h | 4 ----
include/trace/ftrace.h | 7 ++-----
kernel/trace/trace_events.c | 6 ------
kernel/trace/trace_export.c | 7 ++-----
4 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 4e4cc28..4cdb3a1 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -495,10 +495,6 @@ enum {
FILTER_TRACE_FN,
};
-#define EVENT_STORAGE_SIZE 128
-extern struct mutex event_storage_mutex;
-extern char event_storage[EVENT_STORAGE_SIZE];
-
extern int trace_event_raw_init(struct ftrace_event_call *call);
extern int trace_define_field(struct ftrace_event_call *call, const char *type,
const char *name, int offset, int size,
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 1a8b28d..487841d 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -310,15 +310,12 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = { \
#undef __array
#define __array(type, item, len) \
do { \
- mutex_lock(&event_storage_mutex); \
BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
- snprintf(event_storage, sizeof(event_storage), \
- "%s[%d]", #type, len); \
- ret = trace_define_field(event_call, event_storage, #item, \
+ char *type_str = #type"["__stringify(len)"]"; \
+ ret = trace_define_field(event_call, type_str, #item, \
offsetof(typeof(field), item), \
sizeof(field.item), \
is_signed_type(type), FILTER_OTHER); \
- mutex_unlock(&event_storage_mutex); \
if (ret) \
return ret; \
} while (0);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e71ffd4..22826c7 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -27,12 +27,6 @@
DEFINE_MUTEX(event_mutex);
-DEFINE_MUTEX(event_storage_mutex);
-EXPORT_SYMBOL_GPL(event_storage_mutex);
-
-char event_storage[EVENT_STORAGE_SIZE];
-EXPORT_SYMBOL_GPL(event_storage);
-
LIST_HEAD(ftrace_events);
static LIST_HEAD(ftrace_common_fields);
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 7c3e3e7..0f9a9fd 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -96,14 +96,11 @@ static void __always_unused ____ftrace_check_##name(void) \
#define __array(type, item, len) \
do { \
BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
- mutex_lock(&event_storage_mutex); \
- snprintf(event_storage, sizeof(event_storage), \
- "%s[%d]", #type, len); \
- ret = trace_define_field(event_call, event_storage, #item, \
+ char *type_str = #type"["__stringify(len)"]"; \
+ ret = trace_define_field(event_call, type_str, #item, \
offsetof(typeof(field), item), \
sizeof(field.item), \
is_signed_type(type), filter_type); \
- mutex_unlock(&event_storage_mutex); \
if (ret) \
return ret; \
} while (0);
--
1.9.0.rc1.175.g0b1dcb5
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-18 19:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 3:51 [PATCH] tracing: Fix array size mismatch in format string Vaibhav Nagarnaik
2014-03-13 0:16 ` Vaibhav Nagarnaik
2014-03-13 0:53 ` Steven Rostedt
2014-03-13 1:13 ` Vaibhav Nagarnaik
2014-03-18 19:08 ` Vaibhav Nagarnaik
-- strict thread matches above, loose matches on Subject: below --
2014-02-13 23:05 Vaibhav Nagarnaik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®