[for-next][PATCH 0/5] tracing: Really, this is the last update for 5.6 before my pull request!

[for-next][PATCH 0/5] tracing: Really, this is the last update for 5.6 before my pull request!

[Steven Rostedt]

Masami Hiramatsu (4):
      bootconfig: Use bootconfig instead of boot config
      bootconfig: Add more parse error messages
      tools/bootconfig: Show the number of bootconfig nodes
      bootconfig: Show the number of nodes on boot message

Steven Rostedt (VMware) (1):
      ftrace: Protect ftrace_graph_hash with ftrace_sync

----
 init/main.c             | 10 ++++++----
 kernel/trace/ftrace.c   | 11 +++++++++--
 kernel/trace/trace.h    |  2 ++
 lib/bootconfig.c        | 21 ++++++++++++++++-----
 tools/bootconfig/main.c |  1 +
 5 files changed, 34 insertions(+), 11 deletions(-)

[for-next][PATCH 1/5] ftrace: Protect ftrace_graph_hash with ftrace_sync

[Steven Rostedt]

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

As function_graph tracer can run when RCU is not "watching", it can not be
protected by synchronize_rcu() it requires running a task on each CPU before
it can be freed. Calling schedule_on_each_cpu(ftrace_sync) needs to be used.

Link: https://lore.kernel.org/r/20200205131110.GT2935@paulmck-ThinkPad-P72

Cc: stable@vger.kernel.org
Fixes: b9b0c831bed26 ("ftrace: Convert graph filter to use hash tables")
Reported-by: "Paul E. McKenney" <paulmck@kernel.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 11 +++++++++--
 kernel/trace/trace.h  |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 481ede3eac13..3f7ee102868a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5867,8 +5867,15 @@ ftrace_graph_release(struct inode *inode, struct file *file)
 
 		mutex_unlock(&graph_lock);
 
-		/* Wait till all users are no longer using the old hash */
-		synchronize_rcu();
+		/*
+		 * We need to do a hard force of sched synchronization.
+		 * This is because we use preempt_disable() to do RCU, but
+		 * the function tracers can be called where RCU is not watching
+		 * (like before user_exit()). We can not rely on the RCU
+		 * infrastructure to do the synchronization, thus we must do it
+		 * ourselves.
+		 */
+		schedule_on_each_cpu(ftrace_sync);
 
 		free_ftrace_hash(old_hash);
 	}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8c52f5de9384..3c75d29bd861 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -979,6 +979,7 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	hash = rcu_dereference_protected(ftrace_graph_hash, !preemptible());
 
@@ -1031,6 +1032,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
 	 * Have to open code "rcu_dereference_sched()" because the
 	 * function graph tracer can be called when RCU is not
 	 * "watching".
+	 * Protected with schedule_on_each_cpu(ftrace_sync)
 	 */
 	notrace_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
 						 !preemptible());
-- 
2.24.1

[for-next][PATCH 2/5] bootconfig: Use bootconfig instead of boot config

[Steven Rostedt]

From: Masami Hiramatsu <mhiramat@kernel.org>

Use "bootconfig" (1 word) instead of "boot config" (2 words)
in the boot message.

Link: http://lkml.kernel.org/r/158091059459.27924.14414336187441539879.stgit@devnote2

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 init/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/init/main.c b/init/main.c
index f174a59d3903..2de2f9f7aab9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -372,7 +372,7 @@ static void __init setup_boot_config(const char *cmdline)
 
 	copy = memblock_alloc(size + 1, SMP_CACHE_BYTES);
 	if (!copy) {
-		pr_err("Failed to allocate memory for boot config\n");
+		pr_err("Failed to allocate memory for bootconfig\n");
 		return;
 	}
 
@@ -380,9 +380,9 @@ static void __init setup_boot_config(const char *cmdline)
 	copy[size] = '\0';
 
 	if (xbc_init(copy) < 0)
-		pr_err("Failed to parse boot config\n");
+		pr_err("Failed to parse bootconfig\n");
 	else {
-		pr_info("Load boot config: %d bytes\n", size);
+		pr_info("Load bootconfig: %d bytes\n", size);
 		/* keys starting with "kernel." are passed via cmdline */
 		extra_command_line = xbc_make_cmdline("kernel");
 		/* Also, "init." keys are init arguments */
-- 
2.24.1

[for-next][PATCH 3/5] bootconfig: Add more parse error messages

[Steven Rostedt]

From: Masami Hiramatsu <mhiramat@kernel.org>

Add more error messages for following cases.
 - Exceeding max number of nodes
 - Config tree data is empty (e.g. comment only)
 - Config data is empty or exceeding max size
 - bootconfig is already initialized

Link: http://lkml.kernel.org/r/158091060401.27924.9024818742827122764.stgit@devnote2

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/bootconfig.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 055014e233a5..a98ae136529c 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -373,7 +373,8 @@ static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
 				sib->next = xbc_node_index(node);
 			}
 		}
-	}
+	} else
+		xbc_parse_error("Too many nodes", data);
 
 	return node;
 }
@@ -657,8 +658,10 @@ static int __init xbc_verify_tree(void)
 	struct xbc_node *n, *m;
 
 	/* Empty tree */
-	if (xbc_node_num == 0)
+	if (xbc_node_num == 0) {
+		xbc_parse_error("Empty config", xbc_data);
 		return -ENOENT;
+	}
 
 	for (i = 0; i < xbc_node_num; i++) {
 		if (xbc_nodes[i].next > xbc_node_num) {
@@ -732,12 +735,17 @@ int __init xbc_init(char *buf)
 	char *p, *q;
 	int ret, c;
 
-	if (xbc_data)
+	if (xbc_data) {
+		pr_err("Error: bootconfig is already initialized.\n");
 		return -EBUSY;
+	}
 
 	ret = strlen(buf);
-	if (ret > XBC_DATA_MAX - 1 || ret == 0)
+	if (ret > XBC_DATA_MAX - 1 || ret == 0) {
+		pr_err("Error: Config data is %s.\n",
+			ret ? "too big" : "empty");
 		return -ERANGE;
+	}
 
 	xbc_data = buf;
 	xbc_data_size = ret + 1;
-- 
2.24.1

[for-next][PATCH 4/5] tools/bootconfig: Show the number of bootconfig nodes

[Steven Rostedt]

From: Masami Hiramatsu <mhiramat@kernel.org>

Show the number of bootconfig nodes when applying new bootconfig to
initrd.

Since there are limitations of bootconfig not only in its filesize,
but also the number of nodes, the number should be shown when applying
so that user can get the feeling of scale of current bootconfig.

Link: http://lkml.kernel.org/r/158091061337.27924.10886706631693823982.stgit@devnote2

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/bootconfig.c        | 5 ++++-
 tools/bootconfig/main.c | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index a98ae136529c..afb2e767e6fe 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -728,7 +728,8 @@ void __init xbc_destroy_all(void)
  *
  * This parses the boot config text in @buf. @buf must be a
  * null terminated string and smaller than XBC_DATA_MAX.
- * Return 0 if succeeded, or -errno if there is any error.
+ * Return the number of stored nodes (>0) if succeeded, or -errno
+ * if there is any error.
  */
 int __init xbc_init(char *buf)
 {
@@ -788,6 +789,8 @@ int __init xbc_init(char *buf)
 
 	if (ret < 0)
 		xbc_destroy_all();
+	else
+		ret = xbc_node_num;
 
 	return ret;
 }
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 91c9a5c0c499..47f488458328 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -268,6 +268,7 @@ int apply_xbc(const char *path, const char *xbc_path)
 		return ret;
 	}
 	printf("Apply %s to %s\n", xbc_path, path);
+	printf("\tNumber of nodes: %d\n", ret);
 	printf("\tSize: %u bytes\n", (unsigned int)size);
 	printf("\tChecksum: %d\n", (unsigned int)csum);
 
-- 
2.24.1

[for-next][PATCH 5/5] bootconfig: Show the number of nodes on boot message

[Steven Rostedt]

From: Masami Hiramatsu <mhiramat@kernel.org>

Show the number of bootconfig nodes on boot message.

Link: http://lkml.kernel.org/r/158091062297.27924.9051634676068550285.stgit@devnote2

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 init/main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 2de2f9f7aab9..491f1cdb3105 100644
--- a/init/main.c
+++ b/init/main.c
@@ -342,6 +342,7 @@ static void __init setup_boot_config(const char *cmdline)
 	char *data, *copy;
 	const char *p;
 	u32 *hdr;
+	int ret;
 
 	p = strstr(cmdline, "bootconfig");
 	if (!p || (p != cmdline && !isspace(*(p-1))) ||
@@ -379,10 +380,11 @@ static void __init setup_boot_config(const char *cmdline)
 	memcpy(copy, data, size);
 	copy[size] = '\0';
 
-	if (xbc_init(copy) < 0)
+	ret = xbc_init(copy);
+	if (ret < 0)
 		pr_err("Failed to parse bootconfig\n");
 	else {
-		pr_info("Load bootconfig: %d bytes\n", size);
+		pr_info("Load bootconfig: %d bytes %d nodes\n", size, ret);
 		/* keys starting with "kernel." are passed via cmdline */
 		extra_command_line = xbc_make_cmdline("kernel");
 		/* Also, "init." keys are init arguments */
-- 
2.24.1

Re: [for-next][PATCH 1/5] ftrace: Protect ftrace_graph_hash with ftrace_sync

[Paul E. McKenney]

On Wed, Feb 05, 2020 at 05:21:11PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> As function_graph tracer can run when RCU is not "watching", it can not be
> protected by synchronize_rcu() it requires running a task on each CPU before
> it can be freed. Calling schedule_on_each_cpu(ftrace_sync) needs to be used.
> 
> Link: https://lore.kernel.org/r/20200205131110.GT2935@paulmck-ThinkPad-P72
> 
> Cc: stable@vger.kernel.org
> Fixes: b9b0c831bed26 ("ftrace: Convert graph filter to use hash tables")
> Reported-by: "Paul E. McKenney" <paulmck@kernel.org>
> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Nice!  If there is much more call for this, perhaps I should take a hint
from the ftrace_sync() comment and add synchronize_rcu_rude().  ;-)

Reviewed-by: "Paul E. McKenney" <paulmck@kernel.org>

> ---
>  kernel/trace/ftrace.c | 11 +++++++++--
>  kernel/trace/trace.h  |  2 ++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 481ede3eac13..3f7ee102868a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5867,8 +5867,15 @@ ftrace_graph_release(struct inode *inode, struct file *file)
>  
>  		mutex_unlock(&graph_lock);
>  
> -		/* Wait till all users are no longer using the old hash */
> -		synchronize_rcu();
> +		/*
> +		 * We need to do a hard force of sched synchronization.
> +		 * This is because we use preempt_disable() to do RCU, but
> +		 * the function tracers can be called where RCU is not watching
> +		 * (like before user_exit()). We can not rely on the RCU
> +		 * infrastructure to do the synchronization, thus we must do it
> +		 * ourselves.
> +		 */
> +		schedule_on_each_cpu(ftrace_sync);
>  
>  		free_ftrace_hash(old_hash);
>  	}
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 8c52f5de9384..3c75d29bd861 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -979,6 +979,7 @@ static inline int ftrace_graph_addr(struct ftrace_graph_ent *trace)
>  	 * Have to open code "rcu_dereference_sched()" because the
>  	 * function graph tracer can be called when RCU is not
>  	 * "watching".
> +	 * Protected with schedule_on_each_cpu(ftrace_sync)
>  	 */
>  	hash = rcu_dereference_protected(ftrace_graph_hash, !preemptible());
>  
> @@ -1031,6 +1032,7 @@ static inline int ftrace_graph_notrace_addr(unsigned long addr)
>  	 * Have to open code "rcu_dereference_sched()" because the
>  	 * function graph tracer can be called when RCU is not
>  	 * "watching".
> +	 * Protected with schedule_on_each_cpu(ftrace_sync)
>  	 */
>  	notrace_hash = rcu_dereference_protected(ftrace_graph_notrace_hash,
>  						 !preemptible());
> -- 
> 2.24.1
> 
>