From: "zhangwei(Jovi)" <jovi.zhangwei@huawei.com>
To: Steven Rostedt <rostedt@goodmis.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] tracing: fix wrong output of the __array field type string
Date: Sat, 27 Jul 2013 14:35:19 +0800 [thread overview]
Message-ID: <51F36A27.9030709@huawei.com> (raw)
A bug was found in events/sched/sched_switch/format.
field:char prev_comm[32]; offset:8; size:16; signed:1;
and prev_comm field is defined as:
__array( char, prev_comm, TASK_COMM_LEN )
Note TASK_COMM_LEN is 16, but the arrary len is 32 in there.
The root cause is we use global temp buffer event_storage as
constant type string in trace_define_field function when
define __array field, this will make old field type string
flushed by new string in event_storage.
In this patch, type string is checked by core_kernel_data firstly,
if it's not core kernel data, then copy the storage.
This patch also kill the global storage event_storage.
events/sched/sched_switch/format changed after patch:
field:char prev_comm[16]; offset:8; size:16; signed:1;
Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
---
include/linux/ftrace_event.h | 2 --
include/trace/ftrace.h | 4 ++--
kernel/trace/trace_events.c | 20 +++++++++++++-------
kernel/trace/trace_export.c | 4 ++--
4 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 4372658..bd91f0e 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -324,8 +324,6 @@ enum {
};
#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,
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index d615f78..c8bd295 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -303,7 +303,8 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = { \
#undef __array
#define __array(type, item, len) \
do { \
- mutex_lock(&event_storage_mutex); \
+ char event_storage[EVENT_STORAGE_SIZE]; \
+ \
BUILD_BUG_ON(len > MAX_FILTER_STR_VAL); \
snprintf(event_storage, sizeof(event_storage), \
"%s[%d]", #type, len); \
@@ -311,7 +312,6 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = { \
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 5f694ce..5827287 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);
@@ -100,7 +94,15 @@ static int __trace_define_field(struct list_head *head, const char *type,
goto err;
field->name = name;
- field->type = type;
+
+ /* for __array field, type is not a constant string */
+ if (!core_kernel_data((unsigned long)type)) {
+ field->type = kstrdup(type, GFP_KERNEL);
+ if (!field->type)
+ goto err;
+ } else {
+ field->type = type;
+ }
if (filter_type == FILTER_OTHER)
field->filter_type = filter_assign_type(type);
@@ -166,6 +168,10 @@ static void trace_destroy_fields(struct ftrace_event_call *call)
head = trace_get_fields(call);
list_for_each_entry_safe(field, next, head, link) {
list_del(&field->link);
+
+ if (!core_kernel_data((unsigned long)field->type))
+ kfree(field->type);
+
kmem_cache_free(field_cachep, field);
}
}
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index d21a746..490eb32 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -95,15 +95,15 @@ static void __always_unused ____ftrace_check_##name(void) \
#undef __array
#define __array(type, item, len) \
do { \
+ char event_storage[EVENT_STORAGE_SIZE]; \
+ \
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, \
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.7.9.7
next reply other threads:[~2013-07-27 6:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-27 6:35 zhangwei(Jovi) [this message]
2013-08-05 3:04 ` [PATCH] tracing: fix wrong output of the __array field type string zhangwei(Jovi)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F36A27.9030709@huawei.com \
--to=jovi.zhangwei@huawei.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.