Remove the @refcnt argument, because it has side-effects, and
arguments with side-effects are not skipped by the jump over
disabled instrumentation, so it adds overhead even when the
tracepoints are disabled.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
include/linux/module.h | 6 ++----
include/trace/events/module.h | 14 +++++++-------
kernel/module.c | 3 +--
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 5e869ff..393ec39 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -460,8 +460,7 @@ static inline void __module_get(struct module *module)
if (module) {
preempt_disable();
__this_cpu_inc(module->refptr->count);
- trace_module_get(module, _THIS_IP_,
- __this_cpu_read(module->refptr->count));
+ trace_module_get(module, _THIS_IP_);
preempt_enable();
}
}
@@ -475,8 +474,7 @@ static inline int try_module_get(struct module *module)
if (likely(module_is_live(module))) {
__this_cpu_inc(module->refptr->count);
- trace_module_get(module, _THIS_IP_,
- __this_cpu_read(module->refptr->count));
+ trace_module_get(module, _THIS_IP_);
}
else
ret = 0;
diff --git a/include/trace/events/module.h b/include/trace/events/module.h
index 4b0f48b..a585f81 100644
--- a/include/trace/events/module.h
+++ b/include/trace/events/module.h
@@ -53,9 +53,9 @@ TRACE_EVENT(module_free,
DECLARE_EVENT_CLASS(module_refcnt,
- TP_PROTO(struct module *mod, unsigned long ip, int refcnt),
+ TP_PROTO(struct module *mod, unsigned long ip),
- TP_ARGS(mod, ip, refcnt),
+ TP_ARGS(mod, ip),
TP_STRUCT__entry(
__field( unsigned long, ip )
@@ -65,7 +65,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
TP_fast_assign(
__entry->ip = ip;
- __entry->refcnt = refcnt;
+ __entry->refcnt = __this_cpu_read(mod->refptr->count);
__assign_str(name, mod->name);
),
@@ -75,16 +75,16 @@ DECLARE_EVENT_CLASS(module_refcnt,
...Use DECLARE_EVENT_CLASS to remove duplicate code: text data bss dec hex filename 23639 6084 8 29731 7423 kernel/signal.o.orig 22727 6084 8 28819 7093 kernel/signal.o 2 events are converted: signal_queue_overflow: signal_overflow_fail, signal_lose_info No functional change. Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> --- include/trace/events/signal.h | 52 +++++++++++++++++----------------------- 1 files changed, 22 insertions(+), 30 deletions(-) diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h index a510b75..814566c 100644 --- a/include/trace/events/signal.h +++ b/include/trace/events/signal.h @@ -100,18 +100,7 @@ TRACE_EVENT(signal_deliver, __entry->sa_handler, __entry->sa_flags) ); -/** - * signal_overflow_fail - called when signal queue is overflow - * @sig: signal number - * @group: signal to process group or not (bool) - * @info: pointer to struct siginfo - * - * Kernel fails to generate 'sig' signal with 'info' siginfo, because - * siginfo queue is overflow, and the signal is dropped. - * 'group' is not 0 if the signal will be sent to a process group. - * 'sig' is always one of RT signals. - */ -TRACE_EVENT(signal_overflow_fail, +DECLARE_EVENT_CLASS(signal_queue_overflow, TP_PROTO(int sig, int group, struct siginfo *info), @@ -135,6 +124,24 @@ TRACE_EVENT(signal_overflow_fail, ); /** + * signal_overflow_fail - called when signal queue is overflow + * @sig: signal number + * @group: signal to process group or not (bool) + * @info: pointer to struct siginfo + * + * Kernel fails to generate 'sig' signal with 'info' siginfo, because + * siginfo queue is overflow, and the signal is dropped. + * 'group' is not 0 if the signal will be sent to a process group. + * 'sig' is always one of RT signals. + */ +DEFINE_EVENT(signal_queue_overflow, signal_overflow_fail, + + TP_PROTO(int sig, int group, struct siginfo *info), + + TP_ARGS(sig, group, ...
Masami, Can I get your Acked-by for this. Thanks, --
Sure, Acked-by: Masami Hiramatsu <mhiramat@redhat.com> -- Masami Hiramatsu e-mail: mhiramat@redhat.com --
Commit-ID: 4bdde044dc36ac7b01f7502394d52619af9d1927 Gitweb: http://git.kernel.org/tip/4bdde044dc36ac7b01f7502394d52619af9d1927 Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Wed, 24 Mar 2010 10:58:05 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Wed, 31 Mar 2010 22:56:54 -0400 tracing: Convert some signal events to DEFINE_TRACE Use DECLARE_EVENT_CLASS to remove duplicate code: text data bss dec hex filename 23639 6084 8 29731 7423 kernel/signal.o.orig 22727 6084 8 28819 7093 kernel/signal.o 2 events are converted: signal_queue_overflow: signal_overflow_fail, signal_lose_info No functional change. Acked-by: Masami Hiramatsu <mhiramat@redhat.com> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4BA97FBD.8070703@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- include/trace/events/signal.h | 52 +++++++++++++++++----------------------- 1 files changed, 22 insertions(+), 30 deletions(-) diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h index a510b75..814566c 100644 --- a/include/trace/events/signal.h +++ b/include/trace/events/signal.h @@ -100,18 +100,7 @@ TRACE_EVENT(signal_deliver, __entry->sa_handler, __entry->sa_flags) ); -/** - * signal_overflow_fail - called when signal queue is overflow - * @sig: signal number - * @group: signal to process group or not (bool) - * @info: pointer to struct siginfo - * - * Kernel fails to generate 'sig' signal with 'info' siginfo, because - * siginfo queue is overflow, and the signal is dropped. - * 'group' is not 0 if the signal will be sent to a process group. - * 'sig' is always one of RT signals. - */ -TRACE_EVENT(signal_overflow_fail, +DECLARE_EVENT_CLASS(signal_queue_overflow, TP_PROTO(int sig, int group, struct siginfo *info), @@ -135,6 +124,24 @@ TRACE_EVENT(signal_overflow_fail, ); /** + * signal_overflow_fail - called when signal queue is ...
Make some comments consistent with the code.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
include/trace/ftrace.h | 33 +++++++++++++++++++--------------
1 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index ea6f9d4..75dd778 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -154,9 +154,11 @@
*
* field = (typeof(field))entry;
*
- * p = get_cpu_var(ftrace_event_seq);
+ * p = &get_cpu_var(ftrace_event_seq);
* trace_seq_init(p);
- * ret = trace_seq_printf(s, <TP_printk> "\n");
+ * ret = trace_seq_printf(s, "%s: ", <call>);
+ * if (ret)
+ * ret = trace_seq_printf(s, <TP_printk> "\n");
* put_cpu();
* if (!ret)
* return TRACE_TYPE_PARTIAL_LINE;
@@ -450,38 +452,38 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
*
* static void ftrace_raw_event_<call>(proto)
* {
+ * struct ftrace_data_offsets_<call> __maybe_unused __data_offsets;
* struct ring_buffer_event *event;
* struct ftrace_raw_<call> *entry; <-- defined in stage 1
* struct ring_buffer *buffer;
* unsigned long irq_flags;
+ * int __data_size;
* int pc;
*
* local_save_flags(irq_flags);
* pc = preempt_count();
*
+ * __data_size = ftrace_get_offsets_<call>(&__data_offsets, args);
+ *
* event = trace_current_buffer_lock_reserve(&buffer,
* event_<call>.id,
- * sizeof(struct ftrace_raw_<call>),
+ * sizeof(*entry) + __data_size,
* irq_flags, pc);
* if (!event)
* return;
* entry = ring_buffer_event_data(event);
*
- * <assign>; <-- Here we assign the entries by the __field and
- * __array macros.
+ * { <assign>; } <-- Here we assign the entries by the __field and
+ * __array macros.
*
- * trace_current_buffer_unlock_commit(buffer, event, irq_flags, pc);
+ * if (!filter_current_check_discard(buffer, event_call, entry, event))
+ * trace_current_buffer_unlock_commit(buffer,
+ * event, irq_flags, pc);
* }
*
* ...Commit-ID: 50354a8a28d0c91695a2d6d25b5a821bfe557a07 Gitweb: http://git.kernel.org/tip/50354a8a28d0c91695a2d6d25b5a821bfe557a07 Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Wed, 24 Mar 2010 10:58:24 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Wed, 31 Mar 2010 22:56:56 -0400 tracing: Update comments Make some comments consistent with the code. Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4BA97FD0.7090202@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- include/trace/ftrace.h | 33 +++++++++++++++++++-------------- 1 files changed, 19 insertions(+), 14 deletions(-) diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h index ea6f9d4..75dd778 100644 --- a/include/trace/ftrace.h +++ b/include/trace/ftrace.h @@ -154,9 +154,11 @@ * * field = (typeof(field))entry; * - * p = get_cpu_var(ftrace_event_seq); + * p = &get_cpu_var(ftrace_event_seq); * trace_seq_init(p); - * ret = trace_seq_printf(s, <TP_printk> "\n"); + * ret = trace_seq_printf(s, "%s: ", <call>); + * if (ret) + * ret = trace_seq_printf(s, <TP_printk> "\n"); * put_cpu(); * if (!ret) * return TRACE_TYPE_PARTIAL_LINE; @@ -450,38 +452,38 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \ * * static void ftrace_raw_event_<call>(proto) * { + * struct ftrace_data_offsets_<call> __maybe_unused __data_offsets; * struct ring_buffer_event *event; * struct ftrace_raw_<call> *entry; <-- defined in stage 1 * struct ring_buffer *buffer; * unsigned long irq_flags; + * int __data_size; * int pc; * * local_save_flags(irq_flags); * pc = preempt_count(); * + * __data_size = ftrace_get_offsets_<call>(&__data_offsets, args); + * * event = trace_current_buffer_lock_reserve(&buffer, * event_<call>.id, - * sizeof(struct ftrace_raw_<call>), + * sizeof(*entry) + __data_size, * irq_flags, pc); * if (!event) * return; * entry = ...
Thanks Li. -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com --
Tested-by: Randy Dunlap <randy.dunlap@oracle.com> -- ~Randy --
Commit-ID: 3656d5431262ca25aba01d08a5b6e1295ab8feeb Gitweb: http://git.kernel.org/tip/3656d5431262ca25aba01d08a5b6e1295ab8feeb Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Wed, 24 Mar 2010 10:57:43 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Fri, 26 Mar 2010 15:30:21 -0400 tracing: Remove side effect from module tracepoints that caused a GPF Remove the @refcnt argument, because it has side-effects, and arguments with side-effects are not skipped by the jump over disabled instrumentation and are executed even when the tracepoint is disabled. This was also causing a GPF as found by Randy Dunlap: Subject: 2.6.33 GP fault only when built with tracing LKML-Reference: <4BA2B69D.3000309@oracle.com> Tested-by: Randy Dunlap <randy.dunlap@oracle.com> Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: stable@kernel.org Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4BA97FA7.6040406@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- include/linux/module.h | 6 ++---- include/trace/events/module.h | 14 +++++++------- kernel/module.c | 3 +-- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 5e869ff..393ec39 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -460,8 +460,7 @@ static inline void __module_get(struct module *module) if (module) { preempt_disable(); __this_cpu_inc(module->refptr->count); - trace_module_get(module, _THIS_IP_, - __this_cpu_read(module->refptr->count)); + trace_module_get(module, _THIS_IP_); preempt_enable(); } } @@ -475,8 +474,7 @@ static inline int try_module_get(struct module *module) if (likely(module_is_live(module))) { __this_cpu_inc(module->refptr->count); - trace_module_get(module, _THIS_IP_, - __this_cpu_read(module->refptr->count)); + trace_module_get(module, _THIS_IP_); } else ret = ...
Steven, how about also merging my patch that address the underlying bug in module.h that cause the GPF in the first place into 2.6.33.x ? Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com --
I must have missed it, can you resend. I was under the impression that this was the fix for the GPF. If it isn't and your patch is the true fix, then perhaps this patch does not need to be applied to 34, and can wait till 35. And we can push your patch instead. Thanks, --
Commit-ID: ae832d1e03ac9bf09fb8a07fb37908ab40c7cd0e Gitweb: http://git.kernel.org/tip/ae832d1e03ac9bf09fb8a07fb37908ab40c7cd0e Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Wed, 24 Mar 2010 10:57:43 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Wed, 31 Mar 2010 22:56:58 -0400 tracing: Remove side effect from module tracepoints that caused a GPF Remove the @refcnt argument, because it has side-effects, and arguments with side-effects are not skipped by the jump over disabled instrumentation and are executed even when the tracepoint is disabled. This was also causing a GPF as found by Randy Dunlap: Subject: 2.6.33 GP fault only when built with tracing LKML-Reference: <4BA2B69D.3000309@oracle.com> Note, the current 2.6.34-rc has a fix for the actual cause of the GPF, but this fixes one of its triggers. Tested-by: Randy Dunlap <randy.dunlap@oracle.com> Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4BA97FA7.6040406@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- include/linux/module.h | 6 ++---- include/trace/events/module.h | 14 +++++++------- kernel/module.c | 3 +-- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 5e869ff..393ec39 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -460,8 +460,7 @@ static inline void __module_get(struct module *module) if (module) { preempt_disable(); __this_cpu_inc(module->refptr->count); - trace_module_get(module, _THIS_IP_, - __this_cpu_read(module->refptr->count)); + trace_module_get(module, _THIS_IP_); preempt_enable(); } } @@ -475,8 +474,7 @@ static inline int try_module_get(struct module *module) if (likely(module_is_live(module))) { __this_cpu_inc(module->refptr->count); - trace_module_get(module, ...
