[PATCH stable 4.4] tracing: Have trace_buffer_unlock_commit() call the _regs version with NULL
From: "Steven Rostedt (Red Hat)" rostedt@goodmis.org
Commit 33fddff24d05d71f97722cb7deec4964d39d10dc upstream
There's no real difference between trace_buffer_unlock_commit() and trace_buffer_unlock_commit_regs() except that the former passes NULL to ftrace_stack_trace() instead of regs. Have the former be a static inline of the latter which passes NULL for regs.
Signed-off-by: Steven Rostedt rostedt@goodmis.org [backport: modify trace_buffer_unlock_commit() in include/linux/trace_events.h] Fixes: c5e0535fe67b ("tracing: Skip more functions when doing stack tracing of events") Signed-off-by: Kefeng Wang wangkefeng.wang@huawei.com --- include/linux/trace_events.h | 13 +++++++++---- kernel/trace/trace.c | 12 ------------ 2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 311176f290b2..3c752a9928e4 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -168,15 +168,20 @@ struct ring_buffer_event * trace_current_buffer_lock_reserve(struct ring_buffer **current_buffer, int type, unsigned long len, unsigned long flags, int pc); -void trace_buffer_unlock_commit(struct trace_array *tr, - struct ring_buffer *buffer, - struct ring_buffer_event *event, - unsigned long flags, int pc); void trace_buffer_unlock_commit_regs(struct trace_array *tr, struct ring_buffer *buffer, struct ring_buffer_event *event, unsigned long flags, int pc, struct pt_regs *regs); + +static inline void trace_buffer_unlock_commit(struct trace_array *tr, + struct ring_buffer *buffer, + struct ring_buffer_event *event, + unsigned long flags, int pc) +{ + trace_buffer_unlock_commit_regs(tr, buffer, event, flags, pc, NULL); +} + void trace_current_buffer_discard_commit(struct ring_buffer *buffer, struct ring_buffer_event *event);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 29fc55b89491..4cd6d4092e67 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -1698,18 +1698,6 @@ __buffer_unlock_commit(struct ring_buffer *buffer, struct ring_buffer_event *eve ring_buffer_unlock_commit(buffer, event); }
-void trace_buffer_unlock_commit(struct trace_array *tr, - struct ring_buffer *buffer, - struct ring_buffer_event *event, - unsigned long flags, int pc) -{ - __buffer_unlock_commit(buffer, event); - - ftrace_trace_stack(tr, buffer, flags, 6, pc, NULL); - ftrace_trace_userstack(buffer, flags, pc); -} -EXPORT_SYMBOL_GPL(trace_buffer_unlock_commit); - static struct ring_buffer *temp_buffer;
struct ring_buffer_event *
On Tue, 12 Nov 2019 14:18:50 +0800 Kefeng Wang wangkefeng.wang@huawei.com wrote:
From: "Steven Rostedt (Red Hat)" rostedt@goodmis.org
Commit 33fddff24d05d71f97722cb7deec4964d39d10dc upstream
There's no real difference between trace_buffer_unlock_commit() and trace_buffer_unlock_commit_regs() except that the former passes NULL to ftrace_stack_trace() instead of regs. Have the former be a static inline of the latter which passes NULL for regs.
Signed-off-by: Steven Rostedt rostedt@goodmis.org [backport: modify trace_buffer_unlock_commit() in include/linux/trace_events.h] Fixes: c5e0535fe67b ("tracing: Skip more functions when doing stack tracing of events") Signed-off-by: Kefeng Wang wangkefeng.wang@huawei.com
I'm not against this backport, but I'd like to know more why you feel this is a stable fix. Just to get a better backtrace? Can you show some examples of the issue you have and how this fixes it?
Thanks!
-- Steve
On 2019/11/12 22:30, Steven Rostedt wrote:
On Tue, 12 Nov 2019 14:18:50 +0800 Kefeng Wang wangkefeng.wang@huawei.com wrote:
From: "Steven Rostedt (Red Hat)" rostedt@goodmis.org
Commit 33fddff24d05d71f97722cb7deec4964d39d10dc upstream
There's no real difference between trace_buffer_unlock_commit() and trace_buffer_unlock_commit_regs() except that the former passes NULL to ftrace_stack_trace() instead of regs. Have the former be a static inline of the latter which passes NULL for regs.
Signed-off-by: Steven Rostedt rostedt@goodmis.org [backport: modify trace_buffer_unlock_commit() in include/linux/trace_events.h] Fixes: c5e0535fe67b ("tracing: Skip more functions when doing stack tracing of events")
^^^^^^^^^^^^ should be 70b3d6c5aa71 in v4.4.x, I will resend a new one if no objection.
Signed-off-by: Kefeng Wang wangkefeng.wang@huawei.com
I'm not against this backport, but I'd like to know more why you feel this is a stable fix. Just to get a better backtrace? Can you show some examples of the issue you have and how this fixes it?
PATCH1: "tracing: Skip more functions when doing stack tracing of events" from v4.8-rc1~111^2~9 PATCH2: "tracing: Have trace_buffer_unlock_commit() call the _regs version with NULL" from tags/v4.7-rc1~72^2~8 PATCH1 is after PATCH2, the "skip" of ftrace_trace_stack() called by trace_buffer_unlock_commit_regs() and trace_buffer_unlock_commit() is all changed. but we only have PATCH1 in 4.4.x, I found this when using stacktrace on arm64,
cd /sys/kernel/debug/tracing echo 1 > options/stacktrace echo 1 > events/kmem/mm_page_alloc/enable cat trace
before ---
sh-781 [004] ...1 31.300106: mm_page_alloc: page=ffffffbdc0246380 pfn=299406 order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE sh-781 [004] ...1 31.301373: <stack trace> => do_page_fault => do_translation_fault => do_mem_abort => el0_da sh-781 [004] ...1 31.303494: mm_page_alloc: page=ffffffbdc021cf00 pfn=296764 order=2 migratetype=0 gfp_flags=GFP_USER|GFP_ZERO|GFP_NOTRACK sh-781 [004] ...1 31.303508: <stack trace> => copy_process.isra.8 => _do_fork => SyS_clone => el0_svc_naked sh-781 [004] ...1 31.303975: mm_page_alloc: page=ffffffbdc021cd40 pfn=296757 order=0 migratetype=0 gfp_flags=GFP_USER|GFP_REPEAT|GFP_ZERO|GFP_NOTRACK sh-781 [004] ...1 31.303987: <stack trace> => pgd_alloc => mm_init.isra.4 => copy_process.isra.8 => _do_fork => SyS_clone => el0_svc_naked after --- sh-780 [002] ...1 14.728783: mm_page_alloc: page=ffffffbdc2e86200 pfn=1024392 order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE sh-780 [002] ...1 14.729985: <stack trace> => __alloc_pages_nodemask => handle_mm_fault => do_page_fault => do_translation_fault => do_mem_abort => el0_da sh-780 [002] ...1 14.731854: mm_page_alloc: page=ffffffbdc021dc00 pfn=296816 order=2 migratetype=0 gfp_flags=GFP_USER|GFP_ZERO|GFP_NOTRACK sh-780 [002] ...1 14.731867: <stack trace> => __alloc_pages_nodemask => alloc_kmem_pages_node => copy_process.isra.8 => _do_fork => SyS_clone => el0_svc_naked sh-780 [002] ...1 14.732069: mm_page_alloc: page=ffffffbdc021d600 pfn=296792 order=0 migratetype=0 gfp_flags=GFP_USER|GFP_REPEAT|GFP_ZERO|GFP_NOTRACK sh-780 [002] ...1 14.732079: <stack trace> => __alloc_pages_nodemask => __get_free_pages => pgd_alloc => mm_init.isra.4 => copy_process.isra.8 => _do_fork => SyS_clone => el0_svc_naked
Thanks!
-- Steve
.
linux-stable-mirror@lists.linaro.org
-
Kefeng Wang -
Steven Rostedt