[for-next][PATCH 0/7] tracing: Fixes for 4.15

[for-next][PATCH 0/7] tracing: Fixes for 4.15

[Steven Rostedt]

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(-)

[for-next][PATCH 1/7] tracing: Fix code comments in trace.c

[Steven Rostedt]

[-- Attachment #1: 0001-tracing-Fix-code-comments-in-trace.c.patch --]
[-- Type: text/plain, Size: 2588 bytes --]

From: Chunyu Hu <chuhu@redhat.com>

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.

Link: http://lkml.kernel.org/r/1508394753-20887-1-git-send-email-chuhu@redhat.com

Fixes: cab5037 ("tracing/ftrace: Enable snapshot function trigger")
Fixes: 886b5b7 ("tracing: remove /debug/tracing/latency_trace")
Signed-off-by: Chunyu Hu <chuhu@redhat.com>
[ Replaced /sys/kernel/debug/tracing with /sys/kerne/tracing ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 73e67b68c53b..5815ec16edd4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -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)
-- 
2.13.2

[for-next][PATCH 2/7] tracing: always define trace_{irq,preempt}_{enable_disable}

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-always-define-trace_-irq-preempt-_-enable_di.patch --]
[-- Type: text/plain, Size: 2144 bytes --]

From: Arnd Bergmann <arnd@arndb.de>

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.

Link: http://lkml.kernel.org/r/20171019083230.2450779-1-arnd@arndb.de

Fixes: d59158162e03 ("tracing: Add support for preempt and irq enable/disable events")
Acked-by: Joel Fernandes <joelaf@google.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/trace/events/preemptirq.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/preemptirq.h b/include/trace/events/preemptirq.h
index f5024c560d8f..9c4eb33c5a1d 100644
--- a/include/trace/events/preemptirq.h
+++ b/include/trace/events/preemptirq.h
@@ -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
-- 
2.13.2

[for-next][PATCH 3/7] tracing: Allocate mask_str buffer dynamically

[Steven Rostedt]

[-- Attachment #1: 0003-tracing-Allocate-mask_str-buffer-dynamically.patch --]
[-- Type: text/plain, Size: 2886 bytes --]

From: Changbin Du <changbin.du@intel.com>

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.

Link: http://lkml.kernel.org/r/1512013183-19107-1-git-send-email-changbin.du@intel.com

Fixes: 36dfe9252bd4c ("ftrace: make use of tracing_cpumask")
Cc: stable@vger.kernel.org
Signed-off-by: Changbin Du <changbin.du@intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5815ec16edd4..9f3f043ba3b7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -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;
-- 
2.13.2

[for-next][PATCH 4/7] tracing: make PREEMPTIRQ_EVENTS depend on TRACING

[Steven Rostedt]

[-- Attachment #1: 0004-tracing-make-PREEMPTIRQ_EVENTS-depend-on-TRACING.patch --]
[-- Type: text/plain, Size: 1194 bytes --]

From: Arnd Bergmann <arnd@arndb.de>

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.

Link: http://lkml.kernel.org/r/20171103104031.270375-1-arnd@arndb.de

Fixes: d59158162e03 ("tracing: Add support for preempt and irq enable/disable events")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index af7dad126c13..904c952ac383 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -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.
-- 
2.13.2

[for-next][PATCH 5/7] ring-buffer: Remove unused function __rb_data_page_index()

[Steven Rostedt]

[-- Attachment #1: 0005-ring-buffer-Remove-unused-function-__rb_data_page_in.patch --]
[-- Type: text/plain, Size: 1134 bytes --]

From: Matthias Kaehlcke <mka@chromium.org>

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]

Link: http://lkml.kernel.org/r/20170518001415.5223-1-mka@chromium.org

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 91874a95060d..c87766c1c204 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -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;
-- 
2.13.2

[for-next][PATCH 6/7] tracing: Pass export pointer as argument to ->write()

[Steven Rostedt]

[-- Attachment #1: 0006-tracing-Pass-export-pointer-as-argument-to-write.patch --]
[-- Type: text/plain, Size: 2255 bytes --]

From: Felipe Balbi <felipe.balbi@linux.intel.com>

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.

Link: http://lkml.kernel.org/r/20170602102025.5140-1-felipe.balbi@linux.intel.com

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 drivers/hwtracing/stm/ftrace.c | 6 ++++--
 include/linux/trace.h          | 2 +-
 kernel/trace/trace.c           | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
index bd126a7c6da2..7da75644c750 100644
--- a/drivers/hwtracing/stm/ftrace.c
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -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)
diff --git a/include/linux/trace.h b/include/linux/trace.h
index d24991c1fef3..b95ffb2188ab 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -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);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 9f3f043ba3b7..59518b8126d0 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -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);
-- 
2.13.2

[for-next][PATCH 7/7] tracing: Have stack trace not record if RCU is not watching

[Steven Rostedt]

[-- Attachment #1: 0007-tracing-Have-stack-trace-not-record-if-RCU-is-not-wa.patch --]
[-- Type: text/plain, Size: 1312 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

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.

Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_stack.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 734accc02418..3c7bfc4bf5e9 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -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);
-- 
2.13.2

Re: [for-next][PATCH 7/7] tracing: Have stack trace not record if RCU is not watching

[Paul E. McKenney]

On Wed, Dec 13, 2017 at 03:47:03PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> 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.
> 
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

> ---
>  kernel/trace/trace_stack.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 734accc02418..3c7bfc4bf5e9 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -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);
> -- 
> 2.13.2
> 
> 

Re: [for-next][PATCH 7/7] tracing: Have stack trace not record if RCU is not watching

[Steven Rostedt]

On Wed, 13 Dec 2017 20:34:34 -0800
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Wed, Dec 13, 2017 at 03:47:03PM -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > 
> > 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.
> > 
> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>  
> 
> Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>

Thanks!

-- Steve