[PATCH 0/3] tracing/boot: bootconfig: Fixes and API name change

[PATCH 0/3] tracing/boot: bootconfig: Fixes and API name change

[Masami Hiramatsu]

Hi,

Here is a series of patches to fix some issues on boot-time tracing
and bootconfig. Yesterday I sent a bugfix(*), and today I found some
other issues.

(*) https://lore.kernel.org/all/163112988361.74896.2267026262061819145.stgit@devnote2/T/#u

This includes following patches.

[1/3] tracing/boot: Fix trace_boot_hist_add_array() to check array is value

[2/3] tracing/boot: Fix to check the histogram control param is a leaf node

[3/3] bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()
 - Rename xbc_node_find_child() to xbc_node_find_subkey() to avoid
   confusing "child" and "subkey" words.



Thank you,

---

Masami Hiramatsu (3):
      tracing/boot: Fix trace_boot_hist_add_array() to check array is value
      tracing/boot: Fix to check the histogram control param is a leaf node
      bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()


 include/linux/bootconfig.h |    4 ++--
 kernel/trace/trace_boot.c  |   37 ++++++++++++++++++-------------------
 lib/bootconfig.c           |    8 ++++----
 3 files changed, 24 insertions(+), 25 deletions(-)

-- 
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/3] tracing/boot: Fix trace_boot_hist_add_array() to check array is value

[Masami Hiramatsu]

trace_boot_hist_add_array() uses the combination of
xbc_node_find_child() and xbc_node_get_child() to get the
child node of the key node. But since it missed to check
the child node is data node or not, user can pass the
subkey node for the array node (anode).
To avoid this issue, check the array node is a data node.
Actually, there is xbc_node_find_value(node, key, vnode),
which ensures the @vnode is a value node, so use it in
trace_boot_hist_add_array() to fix this issue.

Fixes: e66ed86ca6c5 ("tracing/boot: Add per-event histogram action options")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_boot.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 388e65d05978..a6be48b24774 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -219,13 +219,12 @@ static int __init
 trace_boot_hist_add_array(struct xbc_node *hnode, char **bufp,
 			  char *end, const char *key)
 {
-	struct xbc_node *knode, *anode;
+	struct xbc_node *anode;
 	const char *p;
 	char sep;
 
-	knode = xbc_node_find_child(hnode, key);
-	if (knode) {
-		anode = xbc_node_get_child(knode);
+	p = xbc_node_find_value(hnode, key, &anode);
+	if (p) {
 		if (!anode) {
 			pr_err("hist.%s requires value(s).\n", key);
 			return -EINVAL;

[PATCH 2/3] tracing/boot: Fix to check the histogram control param is a leaf node

[Masami Hiramatsu]

Since xbc_node_find_child() doesn't ensure the returned node
is a leaf node (key-value pair or do not have subkeys),
use xbc_node_find_value to ensure the histogram control
parameter is a leaf node in trace_boot_compose_hist_cmd().

Fixes: e66ed86ca6c5 ("tracing/boot: Add per-event histogram action options")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_boot.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index a6be48b24774..db6ee372dc6d 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -385,11 +385,11 @@ trace_boot_compose_hist_cmd(struct xbc_node *hnode, char *buf, size_t size)
 	}
 
 	/* Histogram control attributes (mutual exclusive) */
-	if (xbc_node_find_child(hnode, "pause"))
+	if (xbc_node_find_value(hnode, "pause", NULL))
 		append_printf(&buf, end, ":pause");
-	else if (xbc_node_find_child(hnode, "continue"))
+	else if (xbc_node_find_value(hnode, "continue", NULL))
 		append_printf(&buf, end, ":continue");
-	else if (xbc_node_find_child(hnode, "clear"))
+	else if (xbc_node_find_value(hnode, "clear", NULL))
 		append_printf(&buf, end, ":clear");
 
 	/* Histogram handler and actions */

[PATCH 3/3] bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()

[Masami Hiramatsu]

Rename xbc_node_find_child() to xbc_node_find_subkey() for
clarifying that function returns a key node (no value node).
Since there are xbc_node_for_each_child() (loop on all child
nodes) and xbc_node_for_each_subkey() (loop on only subkey
nodes), this name distinction is necessary to avoid confusing
users.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/bootconfig.h |    4 ++--
 kernel/trace/trace_boot.c  |   24 ++++++++++++------------
 lib/bootconfig.c           |    8 ++++----
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index abe089c27529..537e1b991f11 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -110,7 +110,7 @@ static inline __init bool xbc_node_is_leaf(struct xbc_node *node)
 }
 
 /* Tree-based key-value access APIs */
-struct xbc_node * __init xbc_node_find_child(struct xbc_node *parent,
+struct xbc_node * __init xbc_node_find_subkey(struct xbc_node *parent,
 					     const char *key);
 
 const char * __init xbc_node_find_value(struct xbc_node *parent,
@@ -148,7 +148,7 @@ xbc_find_value(const char *key, struct xbc_node **vnode)
  */
 static inline struct xbc_node * __init xbc_find_node(const char *key)
 {
-	return xbc_node_find_child(NULL, key);
+	return xbc_node_find_subkey(NULL, key);
 }
 
 /**
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index db6ee372dc6d..8d252f63cd78 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -262,9 +262,9 @@ trace_boot_hist_add_one_handler(struct xbc_node *hnode, char **bufp,
 	append_printf(bufp, end, ":%s(%s)", handler, p);
 
 	/* Compose 'action' parameter */
-	knode = xbc_node_find_child(hnode, "trace");
+	knode = xbc_node_find_subkey(hnode, "trace");
 	if (!knode)
-		knode = xbc_node_find_child(hnode, "save");
+		knode = xbc_node_find_subkey(hnode, "save");
 
 	if (knode) {
 		anode = xbc_node_get_child(knode);
@@ -283,7 +283,7 @@ trace_boot_hist_add_one_handler(struct xbc_node *hnode, char **bufp,
 				sep = ',';
 		}
 		append_printf(bufp, end, ")");
-	} else if (xbc_node_find_child(hnode, "snapshot")) {
+	} else if (xbc_node_find_subkey(hnode, "snapshot")) {
 		append_printf(bufp, end, ".snapshot()");
 	} else {
 		pr_err("hist.%s requires an action.\n",
@@ -314,7 +314,7 @@ trace_boot_hist_add_handlers(struct xbc_node *hnode, char **bufp,
 			break;
 	}
 
-	if (xbc_node_find_child(hnode, param))
+	if (xbc_node_find_subkey(hnode, param))
 		ret = trace_boot_hist_add_one_handler(hnode, bufp, end, handler, param);
 
 	return ret;
@@ -374,7 +374,7 @@ trace_boot_compose_hist_cmd(struct xbc_node *hnode, char *buf, size_t size)
 	if (p)
 		append_printf(&buf, end, ":name=%s", p);
 
-	node = xbc_node_find_child(hnode, "var");
+	node = xbc_node_find_subkey(hnode, "var");
 	if (node) {
 		xbc_node_for_each_key_value(node, knode, p) {
 			/* Expression must not include spaces. */
@@ -393,13 +393,13 @@ trace_boot_compose_hist_cmd(struct xbc_node *hnode, char *buf, size_t size)
 		append_printf(&buf, end, ":clear");
 
 	/* Histogram handler and actions */
-	node = xbc_node_find_child(hnode, "onmax");
+	node = xbc_node_find_subkey(hnode, "onmax");
 	if (node && trace_boot_hist_add_handlers(node, &buf, end, "var") < 0)
 		return -EINVAL;
-	node = xbc_node_find_child(hnode, "onchange");
+	node = xbc_node_find_subkey(hnode, "onchange");
 	if (node && trace_boot_hist_add_handlers(node, &buf, end, "var") < 0)
 		return -EINVAL;
-	node = xbc_node_find_child(hnode, "onmatch");
+	node = xbc_node_find_subkey(hnode, "onmatch");
 	if (node && trace_boot_hist_add_handlers(node, &buf, end, "event") < 0)
 		return -EINVAL;
 
@@ -436,7 +436,7 @@ trace_boot_init_histograms(struct trace_event_file *file,
 		}
 	}
 
-	if (xbc_node_find_child(hnode, "keys")) {
+	if (xbc_node_find_subkey(hnode, "keys")) {
 		if (trace_boot_compose_hist_cmd(hnode, buf, size) == 0) {
 			tmp = kstrdup(buf, GFP_KERNEL);
 			if (trigger_process_regex(file, buf) < 0)
@@ -495,7 +495,7 @@ trace_boot_init_one_event(struct trace_array *tr, struct xbc_node *gnode,
 			else if (trigger_process_regex(file, buf) < 0)
 				pr_err("Failed to apply an action: %s\n", p);
 		}
-		anode = xbc_node_find_child(enode, "hist");
+		anode = xbc_node_find_subkey(enode, "hist");
 		if (anode)
 			trace_boot_init_histograms(file, anode, buf, ARRAY_SIZE(buf));
 	} else if (xbc_node_find_value(enode, "actions", NULL))
@@ -517,7 +517,7 @@ trace_boot_init_events(struct trace_array *tr, struct xbc_node *node)
 	bool enable, enable_all = false;
 	const char *data;
 
-	node = xbc_node_find_child(node, "event");
+	node = xbc_node_find_subkey(node, "event");
 	if (!node)
 		return;
 	/* per-event key starts with "event.GROUP.EVENT" */
@@ -620,7 +620,7 @@ trace_boot_init_instances(struct xbc_node *node)
 	struct trace_array *tr;
 	const char *p;
 
-	node = xbc_node_find_child(node, "instance");
+	node = xbc_node_find_subkey(node, "instance");
 	if (!node)
 		return;
 
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 927017431fb6..f8419cff1147 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -142,16 +142,16 @@ xbc_node_match_prefix(struct xbc_node *node, const char **prefix)
 }
 
 /**
- * xbc_node_find_child() - Find a child node which matches given key
+ * xbc_node_find_subkey() - Find a subkey node which matches given key
  * @parent: An XBC node.
  * @key: A key string.
  *
- * Search a node under @parent which matches @key. The @key can contain
+ * Search a key node under @parent which matches @key. The @key can contain
  * several words jointed with '.'. If @parent is NULL, this searches the
  * node from whole tree. Return NULL if no node is matched.
  */
 struct xbc_node * __init
-xbc_node_find_child(struct xbc_node *parent, const char *key)
+xbc_node_find_subkey(struct xbc_node *parent, const char *key)
 {
 	struct xbc_node *node;
 
@@ -191,7 +191,7 @@ const char * __init
 xbc_node_find_value(struct xbc_node *parent, const char *key,
 		    struct xbc_node **vnode)
 {
-	struct xbc_node *node = xbc_node_find_child(parent, key);
+	struct xbc_node *node = xbc_node_find_subkey(parent, key);
 
 	if (!node || !xbc_node_is_key(node))
 		return NULL;

[PATCH 0/3] tracing/boot: bootconfig: Fixes and API name change

[Masami Hiramatsu]

Hi,

Here is a series of patches to fix some issues on boot-time tracing
and bootconfig. Yesterday I sent a bugfix(*), and today I found some
other issues.

(*) https://lore.kernel.org/all/163112988361.74896.2267026262061819145.stgit@devnote2/T/#u

This includes following patches.

[1/3] tracing/boot: Fix trace_boot_hist_add_array() to check array is value

[2/3] tracing/boot: Fix to check the histogram control param is a leaf node

[3/3] bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()
 - Rename xbc_node_find_child() to xbc_node_find_subkey() to avoid
   confusing "child" and "subkey" words.



Thank you,

---

Masami Hiramatsu (3):
      tracing/boot: Fix trace_boot_hist_add_array() to check array is value
      tracing/boot: Fix to check the histogram control param is a leaf node
      bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()


 include/linux/bootconfig.h |    4 ++--
 kernel/trace/trace_boot.c  |   35 +++++++++++++++++------------------
 lib/bootconfig.c           |    8 ++++----
 3 files changed, 23 insertions(+), 24 deletions(-)

-- 
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

Re: [PATCH 0/3] tracing/boot: bootconfig: Fixes and API name change

[Steven Rostedt]

On Thu,  9 Sep 2021 21:48:43 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here is a series of patches to fix some issues on boot-time tracing
> and bootconfig. Yesterday I sent a bugfix(*), and today I found some
> other issues.
> 
> (*) https://lore.kernel.org/all/163112988361.74896.2267026262061819145.stgit@devnote2/T/#u
> 
> This includes following patches.
> 
> [1/3] tracing/boot: Fix trace_boot_hist_add_array() to check array is value
> 
> [2/3] tracing/boot: Fix to check the histogram control param is a leaf node
> 
> [3/3] bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()
>  - Rename xbc_node_find_child() to xbc_node_find_subkey() to avoid
>    confusing "child" and "subkey" words.
> 

Masami,

Did this go out correctly?

I only received this email, and this email is not showing up on LKML.

 (https://lore.kernel.org/r/163119172318.151131.7138254940364392100.stgit%40devnote2
 gives "Not found")

-- Steve


> 
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (3):
>       tracing/boot: Fix trace_boot_hist_add_array() to check array is value
>       tracing/boot: Fix to check the histogram control param is a leaf node
>       bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()
> 
> 
>  include/linux/bootconfig.h |    4 ++--
>  kernel/trace/trace_boot.c  |   35 +++++++++++++++++------------------
>  lib/bootconfig.c           |    8 ++++----
>  3 files changed, 23 insertions(+), 24 deletions(-)
> 

Re: [PATCH 0/3] tracing/boot: bootconfig: Fixes and API name change

[Masami Hiramatsu]

On Thu, 9 Sep 2021 09:11:23 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu,  9 Sep 2021 21:48:43 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Hi,
> > 
> > Here is a series of patches to fix some issues on boot-time tracing
> > and bootconfig. Yesterday I sent a bugfix(*), and today I found some
> > other issues.
> > 
> > (*) https://lore.kernel.org/all/163112988361.74896.2267026262061819145.stgit@devnote2/T/#u
> > 
> > This includes following patches.
> > 
> > [1/3] tracing/boot: Fix trace_boot_hist_add_array() to check array is value
> > 
> > [2/3] tracing/boot: Fix to check the histogram control param is a leaf node
> > 
> > [3/3] bootconfig: Rename xbc_node_find_child() to xbc_node_find_subkey()
> >  - Rename xbc_node_find_child() to xbc_node_find_subkey() to avoid
> >    confusing "child" and "subkey" words.
> > 
> 
> Masami,
> 
> Did this go out correctly?
> 
> I only received this email, and this email is not showing up on LKML.
> 
>  (https://lore.kernel.org/r/163119172318.151131.7138254940364392100.stgit%40devnote2
>  gives "Not found")

Oops, please ignore this. Maybe something went wrong. 
Hmm, OK, let me resend it with [RESEND] tag.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>