Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt rost...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, 12 Mar 2014 17:16:20 -0700 Vaibhav Nagarnaik 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing: Fix array size mismatch in format string
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
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
> ---
> * 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(_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(_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
Re: [PATCH] tracing: Fix array size mismatch in format string
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
vnagarn...@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 vnagarn...@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)
Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, 12 Mar 2014 17:16:20 -0700 Vaibhav Nagarnaik vnagarn...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tracing: Fix array size mismatch in format string
On Wed, Mar 12, 2014 at 5:53 PM, Steven Rostedt rost...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
