[for-next][PATCH 0/7] tracing: Some very old (and some new) patches for 5.6

[for-next][PATCH 0/7] tracing: Some very old (and some new) patches for 5.6

[Steven Rostedt]

Dan Carpenter (1):
      tracing: Remove unneeded NULL check

Hou Pengyang (1):
      tracing: Fix comments about trace/ftrace.h

Josef Bacik (1):
      tracing: Set kernel_stack's caller size properly

Luis Henriques (1):
      tracing: Fix tracing_stat return values in error handling paths

Steven Rostedt (VMware) (3):
      tracing: Fix very unlikely race of registering two stat tracers
      tracing: Decrement trace_array when bootconfig creates an instance
      tracing: Use pr_err() instead of WARN() for memory failures

----
 include/trace/trace_events.h |  9 ++++++---
 kernel/trace/ftrace.c        |  4 ++--
 kernel/trace/trace.c         | 24 ++++++++++++++----------
 kernel/trace/trace.h         | 12 ++++++++++++
 kernel/trace/trace_boot.c    |  1 +
 kernel/trace/trace_entries.h |  2 +-
 kernel/trace/trace_stat.c    | 31 +++++++++++++++++--------------
 7 files changed, 53 insertions(+), 30 deletions(-)

[for-next][PATCH 1/7] tracing: Fix very unlikely race of registering two stat tracers

[Steven Rostedt]

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

Looking through old emails in my INBOX, I came across a patch from Luis
Henriques that attempted to fix a race of two stat tracers registering the
same stat trace (extremely unlikely, as this is done in the kernel, and
probably doesn't even exist). The submitted patch wasn't quite right as it
needed to deal with clean up a bit better (if two stat tracers were the
same, it would have the same files).

But to make the code cleaner, all we needed to do is to keep the
all_stat_sessions_mutex held for most of the registering function.

Link: http://lkml.kernel.org/r/1410299375-20068-1-git-send-email-luis.henriques@canonical.com

Fixes: 002bb86d8d42f ("tracing/ftrace: separate events tracing and stats tracing engine")
Reported-by: Luis Henriques <luis.henriques@canonical.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_stat.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index 874f1274cf99..da8a38c3d5e4 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -304,7 +304,7 @@ static int init_stat_file(struct stat_session *session)
 int register_stat_tracer(struct tracer_stat *trace)
 {
 	struct stat_session *session, *node;
-	int ret;
+	int ret = -EINVAL;
 
 	if (!trace)
 		return -EINVAL;
@@ -315,17 +315,15 @@ int register_stat_tracer(struct tracer_stat *trace)
 	/* Already registered? */
 	mutex_lock(&all_stat_sessions_mutex);
 	list_for_each_entry(node, &all_stat_sessions, session_list) {
-		if (node->ts == trace) {
-			mutex_unlock(&all_stat_sessions_mutex);
-			return -EINVAL;
-		}
+		if (node->ts == trace)
+			goto out;
 	}
-	mutex_unlock(&all_stat_sessions_mutex);
 
+	ret = -ENOMEM;
 	/* Init the session */
 	session = kzalloc(sizeof(*session), GFP_KERNEL);
 	if (!session)
-		return -ENOMEM;
+		goto out;
 
 	session->ts = trace;
 	INIT_LIST_HEAD(&session->session_list);
@@ -334,15 +332,16 @@ int register_stat_tracer(struct tracer_stat *trace)
 	ret = init_stat_file(session);
 	if (ret) {
 		destroy_session(session);
-		return ret;
+		goto out;
 	}
 
+	ret = 0;
 	/* Register */
-	mutex_lock(&all_stat_sessions_mutex);
 	list_add_tail(&session->session_list, &all_stat_sessions);
+ out:
 	mutex_unlock(&all_stat_sessions_mutex);
 
-	return 0;
+	return ret;
 }
 
 void unregister_stat_tracer(struct tracer_stat *trace)
-- 
2.24.1

[for-next][PATCH 2/7] tracing: Fix tracing_stat return values in error handling paths

[Steven Rostedt]

From: Luis Henriques <luis.henriques@canonical.com>

tracing_stat_init() was always returning '0', even on the error paths.  It
now returns -ENODEV if tracing_init_dentry() fails or -ENOMEM if it fails
to created the 'trace_stat' debugfs directory.

Link: http://lkml.kernel.org/r/1410299381-20108-1-git-send-email-luis.henriques@canonical.com

Fixes: ed6f1c996bfe4 ("tracing: Check return value of tracing_init_dentry()")
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
[ Pulled from the archeological digging of my INBOX ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_stat.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index da8a38c3d5e4..d1fa19773cc8 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -280,18 +280,22 @@ static int tracing_stat_init(void)
 
 	d_tracing = tracing_init_dentry();
 	if (IS_ERR(d_tracing))
-		return 0;
+		return -ENODEV;
 
 	stat_dir = tracefs_create_dir("trace_stat", d_tracing);
-	if (!stat_dir)
+	if (!stat_dir) {
 		pr_warn("Could not create tracefs 'trace_stat' entry\n");
+		return -ENOMEM;
+	}
 	return 0;
 }
 
 static int init_stat_file(struct stat_session *session)
 {
-	if (!stat_dir && tracing_stat_init())
-		return -ENODEV;
+	int ret;
+
+	if (!stat_dir && (ret = tracing_stat_init()))
+		return ret;
 
 	session->file = tracefs_create_file(session->ts->name, 0644,
 					    stat_dir,
-- 
2.24.1

[for-next][PATCH 3/7] tracing: Set kernel_stacks caller size properly

[Steven Rostedt]

From: Josef Bacik <jbacik@fb.com>

I noticed when trying to use the trace-cmd python interface that reading the raw
buffer wasn't working for kernel_stack events.  This is because it uses a
stubbed version of __dynamic_array that doesn't do the __data_loc trick and
encode the length of the array into the field.  Instead it just shows up as a
size of 0.  So change this to __array and set the len to FTRACE_STACK_ENTRIES
since this is what we actually do in practice and matches how user_stack_trace
works.

Link: http://lkml.kernel.org/r/1411589652-1318-1-git-send-email-jbacik@fb.com

Signed-off-by: Josef Bacik <jbacik@fb.com>
[ Pulled from the archeological digging of my INBOX ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_entries.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index fc8e97328e54..78c146efb862 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -174,7 +174,7 @@ FTRACE_ENTRY(kernel_stack, stack_entry,
 
 	F_STRUCT(
 		__field(	int,		size	)
-		__dynamic_array(unsigned long,	caller	)
+		__array(	unsigned long,	caller,	FTRACE_STACK_ENTRIES	)
 	),
 
 	F_printk("\t=> %ps\n\t=> %ps\n\t=> %ps\n"
-- 
2.24.1

[for-next][PATCH 4/7] tracing: Remove unneeded NULL check

[Steven Rostedt]

From: Dan Carpenter <dan.carpenter@oracle.com>

We checked "iter->trace" earlier so there is no need to check here.

Link: http://lkml.kernel.org/r/20141122183012.GB6994@mwanda

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
[ Pulled from the archeological digging of my INBOX ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d1410b4462ac..6fed9b0a8d58 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4224,7 +4224,7 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot)
 	mutex_init(&iter->mutex);
 
 	/* Notify the tracer early; before we stop tracing. */
-	if (iter->trace && iter->trace->open)
+	if (iter->trace->open)
 		iter->trace->open(iter);
 
 	/* Annotate start of buffers if we had overruns */
-- 
2.24.1

[for-next][PATCH 5/7] tracing: Fix comments about trace/ftrace.h

[Steven Rostedt]

From: Hou Pengyang <houpengyang@huawei.com>

commit f42c85e74faa422cf0bc747ed808681145448f88 moved tracepoint's ftrace
creation into include/trace/ftrace.h and trace/define_trace.h was deleted
as a result. However some comment info does not adapt to the change, which
is such a misguiding when reading related code.

This patch fix this by moving trace/trace_events.h to <trace/events/XXX.h>,
since tracepoint headers have already been moved to tarce/events/.

Link: http://lkml.kernel.org/r/1425419298-61941-1-git-send-email-houpengyang@huawei.com

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
[ Pulled from the archeological digging of my INBOX ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/trace/trace_events.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 13a58d453992..831048507fef 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -2,7 +2,8 @@
 /*
  * Stage 1 of the trace events.
  *
- * Override the macros in <trace/trace_events.h> to include the following:
+ * Override the macros in the event tracepoint header <trace/events/XXX.h>
+ * to include the following:
  *
  * struct trace_event_raw_<call> {
  *	struct trace_entry		ent;
@@ -223,7 +224,8 @@ TRACE_MAKE_SYSTEM_STR();
 /*
  * Stage 3 of the trace events.
  *
- * Override the macros in <trace/trace_events.h> to include the following:
+ * Override the macros in the event tracepoint header <trace/events/XXX.h>
+ * to include the following:
  *
  * enum print_line_t
  * trace_raw_output_<call>(struct trace_iterator *iter, int flags)
@@ -555,7 +557,8 @@ static inline notrace int trace_event_get_offsets_##call(		\
 /*
  * Stage 4 of the trace events.
  *
- * Override the macros in <trace/trace_events.h> to include the following:
+ * Override the macros in the event tracepoint header <trace/events/XXX.h>
+ * to include the following:
  *
  * For those macros defined with TRACE_EVENT:
  *
-- 
2.24.1

[for-next][PATCH 6/7] tracing: Decrement trace_array when bootconfig creates an instance

[Steven Rostedt]

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

The trace_array_get_by_name() creates a ftrace instance and
trace_array_put() is used to remove the reference. Even though the
trace_array_get_by_name() creates the instance, it also adds a reference
count to it, that prevents user space from removing it.

As the bootconfig just creates the instance on boot up, it should still be
used where it can be deleted by user space after boot. A trace_array_put()
is required to let that happen.

Also, change the documentation on trace_array_get_by_name() to make this not
be so confusing.

Link: https://lore.kernel.org/r/20200124205927.76128804@rorschach.local.home

Fixes: 4f712a4d04a4e ("tracing/boot: Add instance node support")
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c      | 4 ++++
 kernel/trace/trace_boot.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6fed9b0a8d58..0a5569b1cace 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8602,6 +8602,10 @@ static int instance_mkdir(const char *name)
  * NOTE: This function increments the reference counter associated with the
  * trace array returned. This makes sure it cannot be freed while in use.
  * Use trace_array_put() once the trace array is no longer needed.
+ * If the trace_array is to be freed, trace_array_destroy() needs to
+ * be called after the trace_array_put(), or simply let user space delete
+ * it from the tracefs instances directory. But until the
+ * trace_array_put() is called, user space can not delete it.
  *
  */
 struct trace_array *trace_array_get_by_name(const char *name)
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index cd541ac1cbc1..2f616cd926b0 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -327,6 +327,7 @@ trace_boot_init_instances(struct xbc_node *node)
 			continue;
 		}
 		trace_boot_init_one_instance(tr, inode);
+		trace_array_put(tr);
 	}
 }
 
-- 
2.24.1

[for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN() for memory failures

[Steven Rostedt]

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

As warnings can trigger panics, especially when "panic_on_warn" is set,
memory failure warnings can cause panics and fail fuzz testers that are
stressing memory.

Create a MEM_FAIL() macro to use instead of WARN() in the tracing code
(perhaps this should be a kernel wide macro?), and use that for memory
failure issues. This should stop failing fuzz tests due to warnings.

Link: https://lore.kernel.org/r/CACT4Y+ZP-7np20GVRu3p+eZys9GPtbu+JpfV+HtsufAzvTgJrg@mail.gmail.com

Suggested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |  4 ++--
 kernel/trace/trace.c  | 18 +++++++++---------
 kernel/trace/trace.h  | 12 ++++++++++++
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5c701765da5b..fdb1a9532420 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5459,7 +5459,7 @@ static void __init set_ftrace_early_graph(char *buf, int enable)
 	struct ftrace_hash *hash;
 
 	hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
-	if (WARN_ON(!hash))
+	if (MEM_FAIL(!hash, "Failed to allocate hash\n"))
 		return;
 
 	while (buf) {
@@ -6591,7 +6591,7 @@ static void add_to_clear_hash_list(struct list_head *clear_list,
 
 	func = kmalloc(sizeof(*func), GFP_KERNEL);
 	if (!func) {
-		WARN_ONCE(1, "alloc failure, ftrace filter could be stale\n");
+		MEM_FAIL(1, "alloc failure, ftrace filter could be stale\n");
 		return;
 	}
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0a5569b1cace..6a28b1b9bf42 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3126,7 +3126,7 @@ static int alloc_percpu_trace_buffer(void)
 	struct trace_buffer_struct *buffers;
 
 	buffers = alloc_percpu(struct trace_buffer_struct);
-	if (WARN(!buffers, "Could not allocate percpu trace_printk buffer"))
+	if (MEM_FAIL(!buffers, "Could not allocate percpu trace_printk buffer"))
 		return -ENOMEM;
 
 	trace_percpu_buffer = buffers;
@@ -7932,7 +7932,7 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu)
 
 	tr->percpu_dir = tracefs_create_dir("per_cpu", d_tracer);
 
-	WARN_ONCE(!tr->percpu_dir,
+	MEM_FAIL(!tr->percpu_dir,
 		  "Could not create tracefs directory 'per_cpu/%d'\n", cpu);
 
 	return tr->percpu_dir;
@@ -8253,7 +8253,7 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer)
 	for (cnt = 0; opts[cnt].name; cnt++) {
 		create_trace_option_file(tr, &topts[cnt], flags,
 					 &opts[cnt]);
-		WARN_ONCE(topts[cnt].entry == NULL,
+		MEM_FAIL(topts[cnt].entry == NULL,
 			  "Failed to create trace option: %s",
 			  opts[cnt].name);
 	}
@@ -8437,7 +8437,7 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
 #ifdef CONFIG_TRACER_MAX_TRACE
 	ret = allocate_trace_buffer(tr, &tr->max_buffer,
 				    allocate_snapshot ? size : 1);
-	if (WARN_ON(ret)) {
+	if (MEM_FAIL(ret, "Failed to allocate trace buffer\n")) {
 		ring_buffer_free(tr->array_buffer.buffer);
 		tr->array_buffer.buffer = NULL;
 		free_percpu(tr->array_buffer.data);
@@ -8726,7 +8726,7 @@ static __init void create_trace_instances(struct dentry *d_tracer)
 	trace_instance_dir = tracefs_create_instance_dir("instances", d_tracer,
 							 instance_mkdir,
 							 instance_rmdir);
-	if (WARN_ON(!trace_instance_dir))
+	if (MEM_FAIL(!trace_instance_dir, "Failed to create instances directory\n"))
 		return;
 }
 
@@ -8796,7 +8796,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
 #endif
 
 	if (ftrace_create_function_files(tr, d_tracer))
-		WARN(1, "Could not allocate function filter files");
+		MEM_FAIL(1, "Could not allocate function filter files");
 
 #ifdef CONFIG_TRACER_SNAPSHOT
 	trace_create_file("snapshot", 0644, d_tracer,
@@ -9348,8 +9348,7 @@ __init static int tracer_alloc_buffers(void)
 
 	/* TODO: make the number of buffers hot pluggable with CPUS */
 	if (allocate_trace_buffers(&global_trace, ring_buf_size) < 0) {
-		printk(KERN_ERR "tracer: failed to allocate ring buffer!\n");
-		WARN_ON(1);
+		MEM_FAIL(1, "tracer: failed to allocate ring buffer!\n");
 		goto out_free_savedcmd;
 	}
 
@@ -9422,7 +9421,8 @@ void __init early_trace_init(void)
 	if (tracepoint_printk) {
 		tracepoint_print_iter =
 			kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
-		if (WARN_ON(!tracepoint_print_iter))
+		if (MEM_FAIL(!tracepoint_print_iter,
+			     "Failed to allocate trace iterator\n"))
 			tracepoint_printk = 0;
 		else
 			static_key_enable(&tracepoint_printk_key.key);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 4812a36affac..6bb64d89c321 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -94,6 +94,18 @@ enum trace_type {
 
 #include "trace_entries.h"
 
+/* Use this for memory failure errors */
+#define MEM_FAIL(condition, fmt, ...) ({			\
+	static bool __section(.data.once) __warned;		\
+	int __ret_warn_once = !!(condition);			\
+								\
+	if (unlikely(__ret_warn_once && !__warned)) {		\
+		__warned = true;				\
+		pr_err("ERROR: " fmt, ##__VA_ARGS__);		\
+	}							\
+	unlikely(__ret_warn_once);				\
+})
+
 /*
  * syscalls are special, and need special handling, this is why
  * they are not included in trace_entries.h
-- 
2.24.1

Re: [for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN() for memory failures

[Joe Perches]

On Sun, 2020-01-26 at 14:19 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> As warnings can trigger panics, especially when "panic_on_warn" is set,
> memory failure warnings can cause panics and fail fuzz testers that are
> stressing memory.
> 
> Create a MEM_FAIL() macro to use instead of WARN() in the tracing code
> (perhaps this should be a kernel wide macro?), and use that for memory
> failure issues. This should stop failing fuzz tests due to warnings.

It looks as if most of these could just be removed instead
as there is an existing dump_stack() on failure.

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
[]
> @@ -5459,7 +5459,7 @@ static void __init set_ftrace_early_graph(char *buf, int enable)
>  	struct ftrace_hash *hash;
>  
>  	hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> -	if (WARN_ON(!hash))
> +	if (MEM_FAIL(!hash, "Failed to allocate hash\n"))
>  		return;

has dump_stack()
 
>  	while (buf) {
> @@ -6591,7 +6591,7 @@ static void add_to_clear_hash_list(struct list_head *clear_list,
>  
>  	func = kmalloc(sizeof(*func), GFP_KERNEL);
>  	if (!func) {
> -		WARN_ONCE(1, "alloc failure, ftrace filter could be stale\n");
> +		MEM_FAIL(1, "alloc failure, ftrace filter could be stale\n");

has dump_stack()

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
[]
> @@ -3126,7 +3126,7 @@ static int alloc_percpu_trace_buffer(void)
>  	struct trace_buffer_struct *buffers;
>  
>  	buffers = alloc_percpu(struct trace_buffer_struct);
> -	if (WARN(!buffers, "Could not allocate percpu trace_printk buffer"))
> +	if (MEM_FAIL(!buffers, "Could not allocate percpu trace_printk buffer"))
>  		return -ENOMEM;

has dump_stack()

>  
>  	trace_percpu_buffer = buffers;
> @@ -7932,7 +7932,7 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu)
>  
>  	tr->percpu_dir = tracefs_create_dir("per_cpu", d_tracer);
>  
> -	WARN_ONCE(!tr->percpu_dir,
> +	MEM_FAIL(!tr->percpu_dir,
>  		  "Could not create tracefs directory 'per_cpu/%d'\n", cpu);

keep?
 
>  	return tr->percpu_dir;
> @@ -8253,7 +8253,7 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer)
>  	for (cnt = 0; opts[cnt].name; cnt++) {
>  		create_trace_option_file(tr, &topts[cnt], flags,
>  					 &opts[cnt]);
> -		WARN_ONCE(topts[cnt].entry == NULL,
> +		MEM_FAIL(topts[cnt].entry == NULL,
>  			  "Failed to create trace option: %s",
>  			  opts[cnt].name);
>  	}
> @@ -8437,7 +8437,7 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
>  #ifdef CONFIG_TRACER_MAX_TRACE
>  	ret = allocate_trace_buffer(tr, &tr->max_buffer,
>  				    allocate_snapshot ? size : 1);
> -	if (WARN_ON(ret)) {
> +	if (MEM_FAIL(ret, "Failed to allocate trace buffer\n")) {
>  		ring_buffer_free(tr->array_buffer.buffer);
>  		tr->array_buffer.buffer = NULL;
>  		free_percpu(tr->array_buffer.data);
> @@ -8726,7 +8726,7 @@ static __init void create_trace_instances(struct dentry *d_tracer)
>  	trace_instance_dir = tracefs_create_instance_dir("instances", d_tracer,
>  							 instance_mkdir,
>  							 instance_rmdir);
> -	if (WARN_ON(!trace_instance_dir))
> +	if (MEM_FAIL(!trace_instance_dir, "Failed to create instances directory\n"))
>  		return;
>  }
>  
> @@ -8796,7 +8796,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
>  #endif
>  
>  	if (ftrace_create_function_files(tr, d_tracer))
> -		WARN(1, "Could not allocate function filter files");
> +		MEM_FAIL(1, "Could not allocate function filter files");
>  
>  #ifdef CONFIG_TRACER_SNAPSHOT
>  	trace_create_file("snapshot", 0644, d_tracer,
> @@ -9348,8 +9348,7 @@ __init static int tracer_alloc_buffers(void)
>  
>  	/* TODO: make the number of buffers hot pluggable with CPUS */
>  	if (allocate_trace_buffers(&global_trace, ring_buf_size) < 0) {
> -		printk(KERN_ERR "tracer: failed to allocate ring buffer!\n");
> -		WARN_ON(1);
> +		MEM_FAIL(1, "tracer: failed to allocate ring buffer!\n");

has dump_stack()

>  		goto out_free_savedcmd;
>  	}
>  
> @@ -9422,7 +9421,8 @@ void __init early_trace_init(void)
>  	if (tracepoint_printk) {
>  		tracepoint_print_iter =
>  			kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
> -		if (WARN_ON(!tracepoint_print_iter))
> +		if (MEM_FAIL(!tracepoint_print_iter,
> +			     "Failed to allocate trace iterator\n"))

has dump_stack()

>  			tracepoint_printk = 0;
>  		else
>  			static_key_enable(&tracepoint_printk_key.key);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 4812a36affac..6bb64d89c321 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -94,6 +94,18 @@ enum trace_type {
>  
>  #include "trace_entries.h"
>  
> +/* Use this for memory failure errors */
> +#define MEM_FAIL(condition, fmt, ...) ({			\
> +	static bool __section(.data.once) __warned;		\
> +	int __ret_warn_once = !!(condition);			\
> +								\
> +	if (unlikely(__ret_warn_once && !__warned)) {		\
> +		__warned = true;				\
> +		pr_err("ERROR: " fmt, ##__VA_ARGS__);		\
> +	}							\
> +	unlikely(__ret_warn_once);				\
> +})
> +
>  /*
>   * syscalls are special, and need special handling, this is why
>   * they are not included in trace_entries.h

Re: [for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN() for memory failures

[Steven Rostedt]

On Sun, 26 Jan 2020 12:38:55 -0800
Joe Perches <joe@perches.com> wrote:

> On Sun, 2020-01-26 at 14:19 -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > 
> > As warnings can trigger panics, especially when "panic_on_warn" is set,
> > memory failure warnings can cause panics and fail fuzz testers that are
> > stressing memory.
> > 
> > Create a MEM_FAIL() macro to use instead of WARN() in the tracing code
> > (perhaps this should be a kernel wide macro?), and use that for memory
> > failure issues. This should stop failing fuzz tests due to warnings.  
> 
> It looks as if most of these could just be removed instead
> as there is an existing dump_stack() on failure.

That sounds more generic. This is specific for my own tracing tests to
look for. As the point is, it is *not* to dump_stack, and still report
the error.

-- Steve

Re: [for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN() for memory failures

[Joe Perches]

On Sun, 2020-01-26 at 15:50 -0500, Steven Rostedt wrote:
> On Sun, 26 Jan 2020 12:38:55 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > On Sun, 2020-01-26 at 14:19 -0500, Steven Rostedt wrote:
> > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > > 
> > > As warnings can trigger panics, especially when "panic_on_warn" is set,
> > > memory failure warnings can cause panics and fail fuzz testers that are
> > > stressing memory.
> > > 
> > > Create a MEM_FAIL() macro to use instead of WARN() in the tracing code
> > > (perhaps this should be a kernel wide macro?), and use that for memory
> > > failure issues. This should stop failing fuzz tests due to warnings.  
> > 
> > It looks as if most of these could just be removed instead
> > as there is an existing dump_stack() on failure.
> 
> That sounds more generic. This is specific for my own tracing tests to
> look for. As the point is, it is *not* to dump_stack, and still report
> the error.

__GFP_NOWARN is available too.

Re: [for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN() for memory failures

[Steven Rostedt]

On Sun, 26 Jan 2020 13:07:36 -0800
Joe Perches <joe@perches.com> wrote:

> > That sounds more generic. This is specific for my own tracing tests to
> > look for. As the point is, it is *not* to dump_stack, and still report
> > the error.  
> 
> __GFP_NOWARN is available too.

I honestly don't care if there's a dump_stack or not. I just removed the
WARN_ON. If the allocation causes a dump_stack() then that's fine, but
I still like to have a bit more information at what failed to allocate,
than just a offset into a function.

The point of this patch was simply to remove WARN_ON() that caused
fuzzers to fail.

-- Steve