tracing: Fixes for 4.15
Subsystems (6)
Steven Rostedt
maintainercover tracing: Fixes for 4.15
This should have gone out a while ago, but due to traveling and system updates of my home office, it was delayed.
-- Steve
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 7cf19b391f18a230dc989928334a9b8d10b1bf51
Arnd Bergmann (2):
tracing: always define trace_{irq,preempt}_{enable_disable}
tracing: make PREEMPTIRQ_EVENTS depend on TRACING
Changbin Du (1):
tracing: Allocate mask_str buffer dynamically
Chunyu Hu (1):
tracing: Fix code comments in trace.c
Felipe Balbi (1):
tracing: Pass export pointer as argument to ->write()
Matthias Kaehlcke (1):
ring-buffer: Remove unused function __rb_data_page_index()
Steven Rostedt (VMware) (1):
tracing: Have stack trace not record if RCU is not watching
----
drivers/hwtracing/stm/ftrace.c | 6 ++++-- include/linux/trace.h | 2 +- include/trace/events/preemptirq.h | 11 +++++++---- kernel/trace/Kconfig | 1 + kernel/trace/ring_buffer.c | 6 ------ kernel/trace/trace.c | 41 ++++++++++++++------------------------- kernel/trace/trace_stack.c | 4 ++++ 7 files changed, 32 insertions(+), 39 deletions(-)
Steven Rostedt
maintainerPATCH 1/7 tracing: Fix code comments in trace.c
From: Chunyu Hu <[email protected]>
Naming in code comments for tracing_snapshot, tracing_snapshot_alloc and trace_pid_filter_add_remove_task don't match the real function names. And latency_trace has been removed from tracing directory. Fix them.
--- kernel/trace/trace.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
kernel/trace/trace.c +5 -5
@@ -362,7 +362,7 @@ trace_ignore_this_task(struct trace_pid_list *filtered_pids, struct task_struct }/** -* trace_pid_filter_add_remove - Add or remove a task from a pid_list +* trace_pid_filter_add_remove_task - Add or remove a task from a pid_list * @pid_list: The list to modify * @self: The current task for fork or NULL for exit * @task: The task to add or remove
@@ -925,7 +925,7 @@ static void tracing_snapshot_instance(struct trace_array *tr) }/** -* trace_snapshot - take a snapshot of the current buffer. +* tracing_snapshot - take a snapshot of the current buffer. * * This causes a swap between the snapshot buffer and the current live * tracing buffer. You can use this to take snapshots of the live
@@ -1004,9 +1004,9 @@ int tracing_alloc_snapshot(void)EXPORT_SYMBOL_GPL (tracing_alloc_snapshot);/** -* trace_snapshot_alloc - allocate and take a snapshot of the current buffer. +* tracing_snapshot_alloc - allocate and take a snapshot of the current buffer. * -* This is similar to trace_snapshot(), but it will allocate the +* This is similar to tracing_snapshot(), but it will allocate the * snapshot buffer if it isn't already allocated. Use this only * where it is safe to sleep, as the allocation may sleep. *
@@ -1303,7 +1303,7 @@ unsigned long __read_mostly tracing_thresh;/* * Copy the new maximum trace into the separate maximum-trace * structure. (this way the maximum trace is permanently saved, -* for later retrieval via /sys/kernel/debug/tracing/latency_trace) +* for later retrieval via /sys/kernel/tracing/tracing_max_latency) */ static void __update_max_tr (struct trace_array *tr,struct task_struct *tsk,int cpu)
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 2/7 tracing: always define trace_{irq,preempt}_{enable_disable}
In reply to Steven RostedtFrom: Arnd Bergmann <[email protected]>
We get a build error in the irqsoff tracer in some configurations:
kernel/trace/trace_irqsoff.c: In function 'trace_preempt_on':kernel/trace/trace_irqsoff.c:855:2: error: implicit declaration of function 'trace_preempt_enable_rcuidle'; did you mean 'trace_irq_enable_rcuidle'? [-Werror=implicit-function-declaration]
trace_preempt_enable_rcuidle(a0, a1);
The problem is that trace_preempt_enable_rcuidle() has different definition based on multiple Kconfig symbols, but not all combinations have a valid definition.
This changes the conditions so that we always get exactly one definition of each of the four tracing macros. I have not tried to verify that these definitions are sensible, but now we can build all randconfig combinations again.
--- include/trace/events/preemptirq.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
include/trace/events/preemptirq.h +7 -4
@@ -56,15 +56,18 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,#include <trace/define_trace.h> -#else /* !CONFIG_PREEMPTIRQ_EVENTS */ +#endif /* !CONFIG_PREEMPTIRQ_EVENTS */ +#if !defined(CONFIG_PREEMPTIRQ_EVENTS ) || defined(CONFIG_PROVE_LOCKING )#define trace_irq_enable (...)#define trace_irq_disable (...) -#define trace_preempt_enable (...) -#define trace_preempt_disable (...)#define trace_irq_enable_rcuidle (...)#define trace_irq_disable_rcuidle (...) +#endif + +#if !defined(CONFIG_PREEMPTIRQ_EVENTS ) || !defined(CONFIG_DEBUG_PREEMPT ) +#define trace_preempt_enable (...) +#define trace_preempt_disable (...)#define trace_preempt_enable_rcuidle (...)#define trace_preempt_disable_rcuidle (...) -#endif
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 3/7 tracing: Allocate mask_str buffer dynamically
In reply to Steven RostedtFrom: Changbin Du <[email protected]>
The default NR_CPUS can be very large, but actual possible nr_cpu_ids usually is very small. For my x86 distribution, the NR_CPUS is 8192 and nr_cpu_ids is 4. About 2 pages are wasted.
Most machines don't have so many CPUs, so define a array with NR_CPUS just wastes memory. So let's allocate the buffer dynamically when need.
With this change, the mutext tracing_cpumask_update_lock also can be removed now, which was used to protect mask_str.
--- kernel/trace/trace.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-)
kernel/trace/trace.c +9 -20
@@ -4178,37 +4178,30 @@ static const struct file_operations show_traces_fops = { .llseek = seq_lseek, }; -/* -* The tracer itself will not take this lock, but still we want -* to provide a consistent cpumask to user-space: -*/ -static DEFINE_MUTEX (tracing_cpumask_update_lock); - -/* -* Temporary storage for the character representation of the -* CPU bitmask (and one more byte for the newline): -*/ -static char mask_str[NR_CPUS +1 ]; -static ssize_t tracing_cpumask_read (struct file *filp,char __user *ubuf,size_t count,loff_t *ppos) {struct trace_array *tr =file_inode (filp)->i_private; +char *mask_str;int len; -mutex_lock (&tracing_cpumask_update_lock); + len =snprintf (NULL ,0 ,"%*pb\n" , +cpumask_pr_args (tr->tracing_cpumask)) +1 ; + mask_str =kmalloc (len,GFP_KERNEL ); +if (!mask_str) +return -ENOMEM ; - len =snprintf (mask_str, count,"%*pb\n" , + len =snprintf (mask_str, len,"%*pb\n" ,cpumask_pr_args (tr->tracing_cpumask));if (len >= count) { count = -EINVAL ; goto out_err; } - count =simple_read_from_buffer (ubuf, count, ppos, mask_str,NR_CPUS +1 ); + count =simple_read_from_buffer (ubuf, count, ppos, mask_str, len); out_err: -mutex_unlock (&tracing_cpumask_update_lock); +kfree (mask_str);return count; }
@@ -4228,8 +4221,6 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,if (err) goto err_unlock; -mutex_lock (&tracing_cpumask_update_lock); -local_irq_disable ();arch_spin_lock (&tr->max_lock);for_each_tracing_cpu (cpu) {
@@ -4252,8 +4243,6 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,local_irq_enable ();cpumask_copy (tr->tracing_cpumask, tracing_cpumask_new); - -mutex_unlock (&tracing_cpumask_update_lock);free_cpumask_var (tracing_cpumask_new);return count;
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 4/7 tracing: make PREEMPTIRQ_EVENTS depend on TRACING
In reply to Steven RostedtFrom: Arnd Bergmann <[email protected]>
When CONFIG_TRACING is disabled, the new preemptirq events tracer produces a build failure:
In file included from kernel/trace/trace_irqsoff.c:17:0:kernel/trace/trace.h: In function 'trace_test_and_set_recursion':kernel/trace/trace.h:542:28: error: 'struct task_struct' has no member named 'trace_recursion'
Adding an explicit dependency avoids the broken configuration.
--- kernel/trace/Kconfig | 1 + 1 file changed, 1 insertion(+)
kernel/trace/Kconfig +1 -0
@@ -164,6 +164,7 @@ config PREEMPTIRQ_EVENTS bool "Enable trace events for preempt and irq disable/enable" select TRACE_IRQFLAGS depends on DEBUG_PREEMPT || !PROVE_LOCKING + depends on TRACING default n help Enable tracing of disable and enable events for preemption and irqs.
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 5/7 ring-buffer: Remove unused function __rb_data_page_index()
In reply to Steven RostedtFrom: Matthias Kaehlcke <[email protected]>
This fixes the following warning when building with clang:
kernel/trace/ring_buffer.c:1842:1: error: unused function
'__rb_data_page_index' [-Werror,-Wunused-function]
--- kernel/trace/ring_buffer.c | 6 ------ 1 file changed, 6 deletions(-)
kernel/trace/ring_buffer.c +0 -6
@@ -1799,12 +1799,6 @@ void ring_buffer_change_overwrite(struct ring_buffer *buffer, int val) }EXPORT_SYMBOL_GPL (ring_buffer_change_overwrite); -static __always_inline void * -__rb_data_page_index (struct buffer_data_page *bpage,unsigned index) -{ -return bpage->data + index; -} - static __always_inline void *__rb_page_index(struct buffer_page *bpage, unsigned index) {return bpage->page->data + index;
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 6/7 tracing: Pass export pointer as argument to ->write()
In reply to Steven RostedtFrom: Felipe Balbi <[email protected]>
By passing an export descriptor to the write function, users don't need to keep a global static pointer and can rely on container_of() to fetch their own structure.
--- drivers/hwtracing/stm/ftrace.c | 6 ++++-- include/linux/trace.h | 2 +- kernel/trace/trace.c | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)
drivers/hwtracing/stm/ftrace.c +4 -2
@@ -42,9 +42,11 @@ static struct stm_ftrace {* @len: length of the data packet */ static void notrace -stm_ftrace_write (const void *buf,unsigned int len) +stm_ftrace_write (struct trace_export *export,const void *buf,unsigned int len) { -stm_source_write (&stm_ftrace.data,STM_FTRACE_CHAN , buf, len); + struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace); + +stm_source_write (&stm->data,STM_FTRACE_CHAN , buf, len); }static int stm_ftrace_link (struct stm_source_data *data)
include/linux/trace.h +1 -1
@@ -18,7 +18,7 @@*/ struct trace_export {struct trace_export __rcu *next; -void (*write)(const void *,unsigned int ); +void (*write)(struct trace_export *,const void *,unsigned int ); };int register_ftrace_export (struct trace_export *export);
kernel/trace/trace.c +1 -1
@@ -2415,7 +2415,7 @@ trace_process_export(struct trace_export *export, entry =ring_buffer_event_data (event); size =ring_buffer_event_length (event); - export->write (entry, size); + export->write (export, entry, size); }static DEFINE_MUTEX (ftrace_export_lock);
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 7/7 tracing: Have stack trace not record if RCU is not watching
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
The stack tracer records a stack dump whenever it sees a stack usage that is more than what it ever saw before. This can happen at any function that is being traced. If it happens when the CPU is going idle (or other strange locations), RCU may not be watching, and in this case, the recording of the stack trace will trigger a warning. There's been lots of efforts to make hacks to allow stack tracing to proceed even if RCU is not watching, but this only causes more issues to appear. Simply do not trace a stack if RCU is not watching. It probably isn't a bad stack anyway.
--- kernel/trace/trace_stack.c | 4 ++++ 1 file changed, 4 insertions(+)
kernel/trace/trace_stack.c +4 -0
@@ -209,6 +209,10 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,if (__this_cpu_read (disable_stack_tracer) !=1 ) goto out; +/* If rcu is not watching, then save stack trace can fail */ +if (!rcu_is_watching ()) + goto out; + ip +=MCOUNT_INSN_SIZE ;check_stack (ip, &stack);
Signature · 1 line
2.13.2
Re: [for-next][PATCH 7/7] tracing: Have stack trace not record if RCU is not watching
Quoted 11 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
The stack tracer records a stack dump whenever it sees a stack usage that is more than what it ever saw before. This can happen at any function that is being traced. If it happens when the CPU is going idle (or other strange locations), RCU may not be watching, and in this case, the recording of the stack trace will trigger a warning. There's been lots of efforts to make hacks to allow stack tracing to proceed even if RCU is not watching, but this only causes more issues to appear. Simply do not trace a stack if RCU is not watching. It probably isn't a bad stack anyway.
Quoted 2 lines
Cc "Paul E. McKenney" <[email protected]>Signed-off-by Steven Rostedt (VMware) <[email protected]>
Quoted 23 lines
--- kernel/trace/trace_stack.c | 4 ++++ 1 file changed, 4 insertions(+)kernel/trace/trace_stack.c +4
-0@@ -209,6 +209,10 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,if (__this_cpu_read (disable_stack_tracer) !=1 ) goto out; +/* If rcu is not watching, then save stack trace can fail */ +if (!rcu_is_watching ()) + goto out; + ip +=MCOUNT_INSN_SIZE ;check_stack (ip, &stack);Signature · 1 line
2.13.2
Steven Rostedt
maintainerRe: [for-next][PATCH 7/7] tracing: Have stack trace not record if RCU is not watching
Quoted 12 lines · On Wed, 13 Dec 2017 20:34:34 -0800 "Paul E. McKenney"…
Quoted 11 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
The stack tracer records a stack dump whenever it sees a stack usage that is more than what it ever saw before. This can happen at any function that is being traced. If it happens when the CPU is going idle (or other strange locations), RCU may not be watching, and in this case, the recording of the stack trace will trigger a warning. There's been lots of efforts to make hacks to allow stack tracing to proceed even if RCU is not watching, but this only causes more issues to appear. Simply do not trace a stack if RCU is not watching. It probably isn't a bad stack anyway.
Quoted 5 lines
Quoted 2 lines
Cc "Paul E. McKenney" <[email protected]>Signed-off-by Steven Rostedt (VMware) <[email protected]>
Thanks!
-- Steve