linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
@ 2021-06-02  8:18 Masami Hiramatsu
  2021-06-02  8:18 ` [PATCH v4 1/6] tools/bootconfig: Fix a build error accroding to undefined fallthrough Masami Hiramatsu
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:18 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Hi,

Here is the 4th version of the series which updates bootconfig to
support mixed subkeys and a value under the same key.
The 3rd version is completely wrong. I missed to picked up the older
(v1) patches to v3. So please ignore v3.

Since the kernel cmdline accepts options like 
"aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
also support it.

Here is the previous series (v2):
  https://lore.kernel.org/lkml/162157886618.78209.11141970623539574861.stgit@devnote2/

In this version, I rebased on top of the latest linus tree and
add the build error fix [1/6](*) and a cleanup patch [6/6].

(*) https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/

Changes in v4
 [1/6]:
     - Added from separated thread. This is a fundamental fix.
 [6/6]:
     - New cleanup patch.


With this series, sub-keys and a value can co-exist under a parent key.
For example, following config is allowed.

 foo = value1
 foo.bar = value2

Note, since there is no syntax to put a raw value directly under a
structured key, you have to define it outside of the brace. For example,

 foo {
     bar = value1
     bar {
         baz = value2
         qux = value3
     }
 }

Also, the order of the value node under a key is fixed. If there
are a value and subkeys, the value is always the first child node
of the key. Thus if user specifies subkeys first, e.g.

 foo.bar = value1
 foo = value2

In the program (and /proc/bootconfig), it will be shown as below

 foo = value2
 foo.bar = value1


Thank you,

---

Masami Hiramatsu (6):
      tools/bootconfig: Fix a build error accroding to undefined fallthrough
      bootconfig: Change array value to use child node
      bootconfig: Support mixing a value and subkeys under a key
      tools/bootconfig: Support mixed value and subkey test cases
      docs: bootconfig: Update for mixing value and subkeys
      bootconfig: Share the checksum function with tools


 Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
 fs/proc/bootconfig.c                               |    2 -
 include/linux/bootconfig.h                         |   58 ++++++++++++++-
 init/main.c                                        |   12 ---
 lib/bootconfig.c                                   |   76 +++++++++++++++-----
 tools/bootconfig/include/linux/bootconfig.h        |    4 +
 tools/bootconfig/main.c                            |   62 +++++++++++-----
 tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
 tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
 tools/bootconfig/samples/bad-override.bconf        |    3 -
 tools/bootconfig/samples/bad-override2.bconf       |    3 -
 tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
 tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
 tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
 tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
 tools/bootconfig/samples/good-mixed-override.bconf |    4 +
 16 files changed, 205 insertions(+), 71 deletions(-)
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 1/6] tools/bootconfig: Fix a build error accroding to undefined fallthrough
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
@ 2021-06-02  8:18 ` Masami Hiramatsu
  2021-06-02  8:18 ` [PATCH v4 2/6] bootconfig: Change array value to use child node Masami Hiramatsu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:18 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Since the "fallthrough" is defined only in the kernel, building
lib/bootconfig.c as a part of user-space tools causes a build
error.

Add a dummy fallthrough to avoid the build error.

Cc: stable@vger.kernel.org
Fixes: 4c1ca831adb1 ("Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/include/linux/bootconfig.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/bootconfig/include/linux/bootconfig.h b/tools/bootconfig/include/linux/bootconfig.h
index 078cbd2ba651..de7f30f99af3 100644
--- a/tools/bootconfig/include/linux/bootconfig.h
+++ b/tools/bootconfig/include/linux/bootconfig.h
@@ -4,4 +4,8 @@
 
 #include "../../../../include/linux/bootconfig.h"
 
+#ifndef fallthrough
+# define fallthrough
+#endif
+
 #endif


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 2/6] bootconfig: Change array value to use child node
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
  2021-06-02  8:18 ` [PATCH v4 1/6] tools/bootconfig: Fix a build error accroding to undefined fallthrough Masami Hiramatsu
@ 2021-06-02  8:18 ` Masami Hiramatsu
  2021-06-02  8:19 ` [PATCH v4 3/6] bootconfig: Support mixing a value and subkeys under a key Masami Hiramatsu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:18 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

It is not possible to put an array value with subkeys under
a key node, because both of subkeys and the array elements
are using "next" field of the xbc_node.

Thus this changes the array values to use "child" field in
the array case. The reason why split this change is to
test it easily.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Fix /proc/bootconfig accroding to Devin's report (Thanks!)
  - Update patch description to explain why, not what.
  - Use xbc_node_is_array() instead of checking field directly.
---
 fs/proc/bootconfig.c       |    2 +-
 include/linux/bootconfig.h |    6 +++---
 lib/bootconfig.c           |   23 +++++++++++++++++++----
 tools/bootconfig/main.c    |    2 +-
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c
index ad31ec4ad627..6d8d4bf20837 100644
--- a/fs/proc/bootconfig.c
+++ b/fs/proc/bootconfig.c
@@ -49,7 +49,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size)
 				else
 					q = '"';
 				ret = snprintf(dst, rest(dst, end), "%c%s%c%s",
-					q, val, q, vnode->next ? ", " : "\n");
+					q, val, q, xbc_node_is_array(vnode) ? ", " : "\n");
 				if (ret < 0)
 					goto out;
 				dst += ret;
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 2696eb0fc149..3178a31fdabc 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -71,7 +71,7 @@ static inline __init bool xbc_node_is_key(struct xbc_node *node)
  */
 static inline __init bool xbc_node_is_array(struct xbc_node *node)
 {
-	return xbc_node_is_value(node) && node->next != 0;
+	return xbc_node_is_value(node) && node->child != 0;
 }
 
 /**
@@ -140,7 +140,7 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  */
 #define xbc_array_for_each_value(anode, value)				\
 	for (value = xbc_node_get_data(anode); anode != NULL ;		\
-	     anode = xbc_node_get_next(anode),				\
+	     anode = xbc_node_get_child(anode),				\
 	     value = anode ? xbc_node_get_data(anode) : NULL)
 
 /**
@@ -171,7 +171,7 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  */
 #define xbc_node_for_each_array_value(node, key, anode, value)		\
 	for (value = xbc_node_find_value(node, key, &anode); value != NULL; \
-	     anode = xbc_node_get_next(anode),				\
+	     anode = xbc_node_get_child(anode),				\
 	     value = anode ? xbc_node_get_data(anode) : NULL)
 
 /**
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 9f8c70a98fcf..44dcdcbd746a 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -367,6 +367,14 @@ static inline __init struct xbc_node *xbc_last_sibling(struct xbc_node *node)
 	return node;
 }
 
+static inline __init struct xbc_node *xbc_last_child(struct xbc_node *node)
+{
+	while (node->child)
+		node = xbc_node_get_child(node);
+
+	return node;
+}
+
 static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
 {
 	struct xbc_node *sib, *node = xbc_add_node(data, flag);
@@ -517,17 +525,20 @@ static int __init xbc_parse_array(char **__v)
 	char *next;
 	int c = 0;
 
+	if (last_parent->child)
+		last_parent = xbc_node_get_child(last_parent);
+
 	do {
 		c = __xbc_parse_value(__v, &next);
 		if (c < 0)
 			return c;
 
-		node = xbc_add_sibling(*__v, XBC_VALUE);
+		node = xbc_add_child(*__v, XBC_VALUE);
 		if (!node)
 			return -ENOMEM;
 		*__v = next;
 	} while (c == ',');
-	node->next = 0;
+	node->child = 0;
 
 	return c;
 }
@@ -615,8 +626,12 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
 
 	if (op == ':' && child) {
 		xbc_init_node(child, v, XBC_VALUE);
-	} else if (!xbc_add_sibling(v, XBC_VALUE))
-		return -ENOMEM;
+	} else {
+		if (op == '+' && child)
+			last_parent = xbc_last_child(child);
+		if (!xbc_add_sibling(v, XBC_VALUE))
+			return -ENOMEM;
+	}
 
 	if (c == ',') {	/* Array */
 		c = xbc_parse_array(&next);
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 7362bef1a368..7a98756e594a 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -27,7 +27,7 @@ static int xbc_show_value(struct xbc_node *node, bool semicolon)
 			q = '\'';
 		else
 			q = '"';
-		printf("%c%s%c%s", q, val, q, node->next ? ", " : eol);
+		printf("%c%s%c%s", q, val, q, xbc_node_is_array(node) ? ", " : eol);
 		i++;
 	}
 	return i;


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 3/6] bootconfig: Support mixing a value and subkeys under a key
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
  2021-06-02  8:18 ` [PATCH v4 1/6] tools/bootconfig: Fix a build error accroding to undefined fallthrough Masami Hiramatsu
  2021-06-02  8:18 ` [PATCH v4 2/6] bootconfig: Change array value to use child node Masami Hiramatsu
@ 2021-06-02  8:19 ` Masami Hiramatsu
  2021-06-02  8:19 ` [PATCH v4 4/6] tools/bootconfig: Support mixed value and subkey test cases Masami Hiramatsu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:19 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Support mixing a value and subkeys under a key. Since kernel cmdline
options will support "aaa.bbb=value1 aaa.bbb.ccc=value2", it is
better that the bootconfig supports such configuration too.

Note that this does not change syntax itself but just accepts
mixed value and subkeys e.g.

key = value1
key.subkey = value2

But this is not accepted;

key {
 value1
 subkey = value2
}

That will make value1 as a subkey.

Also, the order of the value node under a key is fixed. If there
are a value and subkeys, the value is always the first child node
of the key. Thus if user specifies subkeys first, e.g.

key.subkey = value1
key = value2

In the program (and /proc/bootconfig), it will be shown as below

key = value2
key.subkey = value1

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/bootconfig.h |   32 ++++++++++++++++++++++
 lib/bootconfig.c           |   65 ++++++++++++++++++++++++++++++--------------
 tools/bootconfig/main.c    |   45 +++++++++++++++++++++++++-----
 3 files changed, 114 insertions(+), 28 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 3178a31fdabc..e49043ac77c9 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -80,6 +80,8 @@ static inline __init bool xbc_node_is_array(struct xbc_node *node)
  *
  * Test the @node is a leaf key node which is a key node and has a value node
  * or no child. Returns true if it is a leaf node, or false if not.
+ * Note that the leaf node can have subkey nodes in addition to the
+ * value node.
  */
 static inline __init bool xbc_node_is_leaf(struct xbc_node *node)
 {
@@ -129,6 +131,23 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
 	return xbc_node_find_child(NULL, key);
 }
 
+/**
+ * xbc_node_get_subkey() - Return the first subkey node if exists
+ * @node: Parent node
+ *
+ * Return the first subkey node of the @node. If the @node has no child
+ * or only value node, this will return NULL.
+ */
+static inline struct xbc_node * __init xbc_node_get_subkey(struct xbc_node *node)
+{
+	struct xbc_node *child = xbc_node_get_child(node);
+
+	if (child && xbc_node_is_value(child))
+		return xbc_node_get_next(child);
+	else
+		return child;
+}
+
 /**
  * xbc_array_for_each_value() - Iterate value nodes on an array
  * @anode: An XBC arraied value node
@@ -149,11 +168,24 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  * @child: Iterated XBC node.
  *
  * Iterate child nodes of @parent. Each child nodes are stored to @child.
+ * The @child can be mixture of a value node and subkey nodes.
  */
 #define xbc_node_for_each_child(parent, child)				\
 	for (child = xbc_node_get_child(parent); child != NULL ;	\
 	     child = xbc_node_get_next(child))
 
+/**
+ * xbc_node_for_each_subkey() - Iterate child subkey nodes
+ * @parent: An XBC node.
+ * @child: Iterated XBC node.
+ *
+ * Iterate subkey nodes of @parent. Each child nodes are stored to @child.
+ * The @child is only the subkey node.
+ */
+#define xbc_node_for_each_subkey(parent, child)				\
+	for (child = xbc_node_get_subkey(parent); child != NULL ;	\
+	     child = xbc_node_get_next(child))
+
 /**
  * xbc_node_for_each_array_value() - Iterate array entries of geven key
  * @node: An XBC node.
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 44dcdcbd746a..927017431fb6 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -156,7 +156,7 @@ xbc_node_find_child(struct xbc_node *parent, const char *key)
 	struct xbc_node *node;
 
 	if (parent)
-		node = xbc_node_get_child(parent);
+		node = xbc_node_get_subkey(parent);
 	else
 		node = xbc_root_node();
 
@@ -164,7 +164,7 @@ xbc_node_find_child(struct xbc_node *parent, const char *key)
 		if (!xbc_node_match_prefix(node, &key))
 			node = xbc_node_get_next(node);
 		else if (*key != '\0')
-			node = xbc_node_get_child(node);
+			node = xbc_node_get_subkey(node);
 		else
 			break;
 	}
@@ -274,6 +274,8 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
 struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 						 struct xbc_node *node)
 {
+	struct xbc_node *next;
+
 	if (unlikely(!xbc_data))
 		return NULL;
 
@@ -282,6 +284,13 @@ struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 		if (!node)
 			node = xbc_nodes;
 	} else {
+		/* Leaf node may have a subkey */
+		next = xbc_node_get_subkey(node);
+		if (next) {
+			node = next;
+			goto found;
+		}
+
 		if (node == root)	/* @root was a leaf, no child node. */
 			return NULL;
 
@@ -296,6 +305,7 @@ struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 		node = xbc_node_get_next(node);
 	}
 
+found:
 	while (node && !xbc_node_is_leaf(node))
 		node = xbc_node_get_child(node);
 
@@ -375,18 +385,20 @@ static inline __init struct xbc_node *xbc_last_child(struct xbc_node *node)
 	return node;
 }
 
-static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
+static struct xbc_node * __init __xbc_add_sibling(char *data, u32 flag, bool head)
 {
 	struct xbc_node *sib, *node = xbc_add_node(data, flag);
 
 	if (node) {
 		if (!last_parent) {
+			/* Ignore @head in this case */
 			node->parent = XBC_NODE_MAX;
 			sib = xbc_last_sibling(xbc_nodes);
 			sib->next = xbc_node_index(node);
 		} else {
 			node->parent = xbc_node_index(last_parent);
-			if (!last_parent->child) {
+			if (!last_parent->child || head) {
+				node->next = last_parent->child;
 				last_parent->child = xbc_node_index(node);
 			} else {
 				sib = xbc_node_get_child(last_parent);
@@ -400,6 +412,16 @@ static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
 	return node;
 }
 
+static inline struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
+{
+	return __xbc_add_sibling(data, flag, false);
+}
+
+static inline struct xbc_node * __init xbc_add_head_sibling(char *data, u32 flag)
+{
+	return __xbc_add_sibling(data, flag, true);
+}
+
 static inline __init struct xbc_node *xbc_add_child(char *data, u32 flag)
 {
 	struct xbc_node *node = xbc_add_sibling(data, flag);
@@ -568,8 +590,9 @@ static int __init __xbc_add_key(char *k)
 		node = find_match_node(xbc_nodes, k);
 	else {
 		child = xbc_node_get_child(last_parent);
+		/* Since the value node is the first child, skip it. */
 		if (child && xbc_node_is_value(child))
-			return xbc_parse_error("Subkey is mixed with value", k);
+			child = xbc_node_get_next(child);
 		node = find_match_node(child, k);
 	}
 
@@ -612,27 +635,29 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
 	if (ret)
 		return ret;
 
-	child = xbc_node_get_child(last_parent);
-	if (child) {
-		if (xbc_node_is_key(child))
-			return xbc_parse_error("Value is mixed with subkey", v);
-		else if (op == '=')
-			return xbc_parse_error("Value is redefined", v);
-	}
-
 	c = __xbc_parse_value(&v, &next);
 	if (c < 0)
 		return c;
 
-	if (op == ':' && child) {
-		xbc_init_node(child, v, XBC_VALUE);
-	} else {
-		if (op == '+' && child)
-			last_parent = xbc_last_child(child);
-		if (!xbc_add_sibling(v, XBC_VALUE))
-			return -ENOMEM;
+	child = xbc_node_get_child(last_parent);
+	if (child && xbc_node_is_value(child)) {
+		if (op == '=')
+			return xbc_parse_error("Value is redefined", v);
+		if (op == ':') {
+			unsigned short nidx = child->next;
+
+			xbc_init_node(child, v, XBC_VALUE);
+			child->next = nidx;	/* keep subkeys */
+			goto array;
+		}
+		/* op must be '+' */
+		last_parent = xbc_last_child(child);
 	}
+	/* The value node should always be the first child */
+	if (!xbc_add_head_sibling(v, XBC_VALUE))
+		return -ENOMEM;
 
+array:
 	if (c == ',') {	/* Array */
 		c = xbc_parse_array(&next);
 		if (c < 0)
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 7a98756e594a..f5b564206428 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -35,30 +35,55 @@ static int xbc_show_value(struct xbc_node *node, bool semicolon)
 
 static void xbc_show_compact_tree(void)
 {
-	struct xbc_node *node, *cnode;
+	struct xbc_node *node, *cnode = NULL, *vnode;
 	int depth = 0, i;
 
 	node = xbc_root_node();
 	while (node && xbc_node_is_key(node)) {
 		for (i = 0; i < depth; i++)
 			printf("\t");
-		cnode = xbc_node_get_child(node);
+		if (!cnode)
+			cnode = xbc_node_get_child(node);
 		while (cnode && xbc_node_is_key(cnode) && !cnode->next) {
+			vnode = xbc_node_get_child(cnode);
+			/*
+			 * If @cnode has value and subkeys, this
+			 * should show it as below.
+			 *
+			 * key(@node) {
+			 *      key(@cnode) = value;
+			 *      key(@cnode) {
+			 *          subkeys;
+			 *      }
+			 * }
+			 */
+			if (vnode && xbc_node_is_value(vnode) && vnode->next)
+				break;
 			printf("%s.", xbc_node_get_data(node));
 			node = cnode;
-			cnode = xbc_node_get_child(node);
+			cnode = vnode;
 		}
 		if (cnode && xbc_node_is_key(cnode)) {
 			printf("%s {\n", xbc_node_get_data(node));
 			depth++;
 			node = cnode;
+			cnode = NULL;
 			continue;
 		} else if (cnode && xbc_node_is_value(cnode)) {
 			printf("%s = ", xbc_node_get_data(node));
 			xbc_show_value(cnode, true);
+			/*
+			 * If @node has value and subkeys, continue
+			 * looping on subkeys with same node.
+			 */
+			if (cnode->next) {
+				cnode = xbc_node_get_next(cnode);
+				continue;
+			}
 		} else {
 			printf("%s;\n", xbc_node_get_data(node));
 		}
+		cnode = NULL;
 
 		if (node->next) {
 			node = xbc_node_get_next(node);
@@ -70,10 +95,12 @@ static void xbc_show_compact_tree(void)
 				return;
 			if (!xbc_node_get_child(node)->next)
 				continue;
-			depth--;
-			for (i = 0; i < depth; i++)
-				printf("\t");
-			printf("}\n");
+			if (depth) {
+				depth--;
+				for (i = 0; i < depth; i++)
+					printf("\t");
+				printf("}\n");
+			}
 		}
 		node = xbc_node_get_next(node);
 	}
@@ -88,8 +115,10 @@ static void xbc_show_list(void)
 
 	xbc_for_each_key_value(leaf, val) {
 		ret = xbc_node_compose_key(leaf, key, XBC_KEYLEN_MAX);
-		if (ret < 0)
+		if (ret < 0) {
+			fprintf(stderr, "Failed to compose key %d\n", ret);
 			break;
+		}
 		printf("%s = ", key);
 		if (!val || val[0] == '\0') {
 			printf("\"\"\n");


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 4/6] tools/bootconfig: Support mixed value and subkey test cases
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2021-06-02  8:19 ` [PATCH v4 3/6] bootconfig: Support mixing a value and subkeys under a key Masami Hiramatsu
@ 2021-06-02  8:19 ` Masami Hiramatsu
  2021-06-02  8:19 ` [PATCH v4 5/6] docs: bootconfig: Update for mixing value and subkeys Masami Hiramatsu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:19 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Update test case to support mixed value and subkey on a key.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 ---
 tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 ---
 tools/bootconfig/samples/bad-override.bconf        |    3 ---
 tools/bootconfig/samples/bad-override2.bconf       |    3 ---
 tools/bootconfig/samples/good-mixed-append.bconf   |    4 ++++
 tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +++
 tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +++
 tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++++++
 tools/bootconfig/samples/good-mixed-override.bconf |    4 ++++
 9 files changed, 20 insertions(+), 12 deletions(-)
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf

diff --git a/tools/bootconfig/samples/bad-mixed-kv1.bconf b/tools/bootconfig/samples/bad-mixed-kv1.bconf
deleted file mode 100644
index 1761547dd05c..000000000000
--- a/tools/bootconfig/samples/bad-mixed-kv1.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-# value -> subkey pattern
-key = value
-key.subkey = another-value
diff --git a/tools/bootconfig/samples/bad-mixed-kv2.bconf b/tools/bootconfig/samples/bad-mixed-kv2.bconf
deleted file mode 100644
index 6b32e0c3878c..000000000000
--- a/tools/bootconfig/samples/bad-mixed-kv2.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-# subkey -> value pattern
-key.subkey = value
-key = another-value
diff --git a/tools/bootconfig/samples/bad-override.bconf b/tools/bootconfig/samples/bad-override.bconf
deleted file mode 100644
index fde6c561512e..000000000000
--- a/tools/bootconfig/samples/bad-override.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-key.subkey = value
-# We can not override pre-defined subkeys with value
-key := value
diff --git a/tools/bootconfig/samples/bad-override2.bconf b/tools/bootconfig/samples/bad-override2.bconf
deleted file mode 100644
index 688587cb023c..000000000000
--- a/tools/bootconfig/samples/bad-override2.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-key = value
-# We can not override pre-defined value with subkey
-key.subkey := value
diff --git a/tools/bootconfig/samples/good-mixed-append.bconf b/tools/bootconfig/samples/good-mixed-append.bconf
new file mode 100644
index 000000000000..b99a089a05f5
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-append.bconf
@@ -0,0 +1,4 @@
+key = foo
+keyx.subkey = value
+key += bar
+
diff --git a/tools/bootconfig/samples/good-mixed-kv1.bconf b/tools/bootconfig/samples/good-mixed-kv1.bconf
new file mode 100644
index 000000000000..1761547dd05c
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv1.bconf
@@ -0,0 +1,3 @@
+# value -> subkey pattern
+key = value
+key.subkey = another-value
diff --git a/tools/bootconfig/samples/good-mixed-kv2.bconf b/tools/bootconfig/samples/good-mixed-kv2.bconf
new file mode 100644
index 000000000000..6b32e0c3878c
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv2.bconf
@@ -0,0 +1,3 @@
+# subkey -> value pattern
+key.subkey = value
+key = another-value
diff --git a/tools/bootconfig/samples/good-mixed-kv3.bconf b/tools/bootconfig/samples/good-mixed-kv3.bconf
new file mode 100644
index 000000000000..2ce2b02224b8
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv3.bconf
@@ -0,0 +1,6 @@
+# mixed key and subkeys with braces
+key = value
+key {
+	subkey1
+	subkey2 = foo
+}
diff --git a/tools/bootconfig/samples/good-mixed-override.bconf b/tools/bootconfig/samples/good-mixed-override.bconf
new file mode 100644
index 000000000000..18195b2873b6
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-override.bconf
@@ -0,0 +1,4 @@
+key.foo = bar
+key = value
+# mixed key value can be overridden
+key := value2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 5/6] docs: bootconfig: Update for mixing value and subkeys
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2021-06-02  8:19 ` [PATCH v4 4/6] tools/bootconfig: Support mixed value and subkey test cases Masami Hiramatsu
@ 2021-06-02  8:19 ` Masami Hiramatsu
  2021-06-02  8:19 ` [PATCH v4 6/6] bootconfig: Share the checksum function with tools Masami Hiramatsu
  2021-06-02 23:02 ` [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Devin Moore
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:19 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Update document for the mixing value and subkeys on a key.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Update an example of mixing value and subkeys.
---
 Documentation/admin-guide/bootconfig.rst |   30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 452b7dcd7f6b..6a79f2e59396 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -89,13 +89,35 @@ you can use ``+=`` operator. For example::
 
 In this case, the key ``foo`` has ``bar``, ``baz`` and ``qux``.
 
-However, a sub-key and a value can not co-exist under a parent key.
-For example, following config is NOT allowed.::
+Moreover, sub-keys and a value can coexist under a parent key.
+For example, following config is allowed.::
 
  foo = value1
- foo.bar = value2 # !ERROR! subkey "bar" and value "value1" can NOT co-exist
- foo.bar := value2 # !ERROR! even with the override operator, this is NOT allowed.
+ foo.bar = value2
+ foo := value3 # This will update foo's value.
+
+Note, since there is no syntax to put a raw value directly under a
+structured key, you have to define it outside of the brace. For example::
+
+ foo {
+     bar = value1
+     bar {
+         baz = value2
+         qux = value3
+     }
+ }
+
+Also, the order of the value node under a key is fixed. If there
+are a value and subkeys, the value is always the first child node
+of the key. Thus if user specifies subkeys first, e.g.::
+
+ foo.bar = value1
+ foo = value2
+
+In the program (and /proc/bootconfig), it will be shown as below::
 
+ foo = value2
+ foo.bar = value1
 
 Comments
 --------


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 6/6] bootconfig: Share the checksum function with tools
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2021-06-02  8:19 ` [PATCH v4 5/6] docs: bootconfig: Update for mixing value and subkeys Masami Hiramatsu
@ 2021-06-02  8:19 ` Masami Hiramatsu
  2021-06-02 23:02 ` [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Devin Moore
  6 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-02  8:19 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, LKML, Ingo Molnar, Devin Moore

Move the checksum calculation function into the header for sharing it
with tools/bootconfig.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/bootconfig.h |   20 ++++++++++++++++++++
 init/main.c                |   12 +-----------
 tools/bootconfig/main.c    |   15 ++-------------
 3 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index e49043ac77c9..6bdd94cff4e2 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -16,6 +16,26 @@
 #define BOOTCONFIG_ALIGN	(1 << BOOTCONFIG_ALIGN_SHIFT)
 #define BOOTCONFIG_ALIGN_MASK	(BOOTCONFIG_ALIGN - 1)
 
+/**
+ * xbc_calc_checksum() - Calculate checksum of bootconfig
+ * @data: Bootconfig data.
+ * @size: The size of the bootconfig data.
+ *
+ * Calculate the checksum value of the bootconfig data.
+ * The checksum will be used with the BOOTCONFIG_MAGIC and the size for
+ * embedding the bootconfig in the initrd image.
+ */
+static inline __init u32 xbc_calc_checksum(void *data, u32 size)
+{
+	unsigned char *p = data;
+	u32 ret = 0;
+
+	while (size--)
+		ret += *p++;
+
+	return ret;
+}
+
 /* XBC tree node */
 struct xbc_node {
 	u16 next;
diff --git a/init/main.c b/init/main.c
index eb01e121d2f1..43914e675421 100644
--- a/init/main.c
+++ b/init/main.c
@@ -386,16 +386,6 @@ static char * __init xbc_make_cmdline(const char *key)
 	return new_cmdline;
 }
 
-static u32 boot_config_checksum(unsigned char *p, u32 size)
-{
-	u32 ret = 0;
-
-	while (size--)
-		ret += *p++;
-
-	return ret;
-}
-
 static int __init bootconfig_params(char *param, char *val,
 				    const char *unused, void *arg)
 {
@@ -439,7 +429,7 @@ static void __init setup_boot_config(void)
 		return;
 	}
 
-	if (boot_config_checksum((unsigned char *)data, size) != csum) {
+	if (xbc_calc_checksum(data, size) != csum) {
 		pr_err("bootconfig checksum failed\n");
 		return;
 	}
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index f5b564206428..483a948af522 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -128,17 +128,6 @@ static void xbc_show_list(void)
 	}
 }
 
-/* Simple real checksum */
-static int checksum(unsigned char *buf, int len)
-{
-	int i, sum = 0;
-
-	for (i = 0; i < len; i++)
-		sum += buf[i];
-
-	return sum;
-}
-
 #define PAGE_SIZE	4096
 
 static int load_xbc_fd(int fd, char **buf, int size)
@@ -234,7 +223,7 @@ static int load_xbc_from_initrd(int fd, char **buf)
 		return ret;
 
 	/* Wrong Checksum */
-	rcsum = checksum((unsigned char *)*buf, size);
+	rcsum = xbc_calc_checksum(*buf, size);
 	if (csum != rcsum) {
 		pr_err("checksum error: %d != %d\n", csum, rcsum);
 		return -EINVAL;
@@ -383,7 +372,7 @@ static int apply_xbc(const char *path, const char *xbc_path)
 		return ret;
 	}
 	size = strlen(buf) + 1;
-	csum = checksum((unsigned char *)buf, size);
+	csum = xbc_calc_checksum(buf, size);
 
 	/* Backup the bootconfig data */
 	data = calloc(size + BOOTCONFIG_ALIGN +


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
                   ` (5 preceding siblings ...)
  2021-06-02  8:19 ` [PATCH v4 6/6] bootconfig: Share the checksum function with tools Masami Hiramatsu
@ 2021-06-02 23:02 ` Devin Moore
  2021-06-10 14:48   ` Masami Hiramatsu
  6 siblings, 1 reply; 17+ messages in thread
From: Devin Moore @ 2021-06-02 23:02 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Steven Rostedt, LKML, Ingo Molnar

On Wed, Jun 2, 2021 at 1:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi,
>
> Here is the 4th version of the series which updates bootconfig to
> support mixed subkeys and a value under the same key.
> The 3rd version is completely wrong. I missed to picked up the older
> (v1) patches to v3. So please ignore v3.
>
> Since the kernel cmdline accepts options like
> "aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
> also support it.
>
> Here is the previous series (v2):
>   https://lore.kernel.org/lkml/162157886618.78209.11141970623539574861.stgit@devnote2/
>
> In this version, I rebased on top of the latest linus tree and
> add the build error fix [1/6](*) and a cleanup patch [6/6].
>
> (*) https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/
>
> Changes in v4
>  [1/6]:
>      - Added from separated thread. This is a fundamental fix.
>  [6/6]:
>      - New cleanup patch.
>
>
> With this series, sub-keys and a value can co-exist under a parent key.
> For example, following config is allowed.
>
>  foo = value1
>  foo.bar = value2
>
> Note, since there is no syntax to put a raw value directly under a
> structured key, you have to define it outside of the brace. For example,
>
>  foo {
>      bar = value1
>      bar {
>          baz = value2
>          qux = value3
>      }
>  }
>
> Also, the order of the value node under a key is fixed. If there
> are a value and subkeys, the value is always the first child node
> of the key. Thus if user specifies subkeys first, e.g.
>
>  foo.bar = value1
>  foo = value2
>
> In the program (and /proc/bootconfig), it will be shown as below
>
>  foo = value2
>  foo.bar = value1
>
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (6):
>       tools/bootconfig: Fix a build error accroding to undefined fallthrough
>       bootconfig: Change array value to use child node
>       bootconfig: Support mixing a value and subkeys under a key
>       tools/bootconfig: Support mixed value and subkey test cases
>       docs: bootconfig: Update for mixing value and subkeys
>       bootconfig: Share the checksum function with tools
>
>
>  Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
>  fs/proc/bootconfig.c                               |    2 -
>  include/linux/bootconfig.h                         |   58 ++++++++++++++-
>  init/main.c                                        |   12 ---
>  lib/bootconfig.c                                   |   76 +++++++++++++++-----
>  tools/bootconfig/include/linux/bootconfig.h        |    4 +
>  tools/bootconfig/main.c                            |   62 +++++++++++-----
>  tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
>  tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
>  tools/bootconfig/samples/bad-override.bconf        |    3 -
>  tools/bootconfig/samples/bad-override2.bconf       |    3 -
>  tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
>  tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
>  tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
>  tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
>  tools/bootconfig/samples/good-mixed-override.bconf |    4 +
>  16 files changed, 205 insertions(+), 71 deletions(-)
>  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-override.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


Thanks for the updated patches.
I tested the v4 changes on an Android virtual device(Cuttlefish) with
a variety of
parameters that included array values. I checked the output in /proc/bootconfig
before and after the changes.
I added a new parameter that failed before these changes and it worked great!
Added 'androidboot.hardware=cutf_cvm ' which worked with
'androidboot.hardware.gralloc = "minigbm"' that was already present.

These changes really help Android simplify some boot configuration
processes. We will be using it ASAP.

So,

Tested-by: Devin Moore <devinmoore@google.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-02 23:02 ` [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Devin Moore
@ 2021-06-10 14:48   ` Masami Hiramatsu
  2021-06-10 16:26     ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-10 14:48 UTC (permalink / raw)
  To: Steven Rostedt, Devin Moore; +Cc: LKML, Ingo Molnar

On Wed, 2 Jun 2021 16:02:27 -0700
Devin Moore <devinmoore@google.com> wrote:

> On Wed, Jun 2, 2021 at 1:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Hi,
> >
> > Here is the 4th version of the series which updates bootconfig to
> > support mixed subkeys and a value under the same key.
> > The 3rd version is completely wrong. I missed to picked up the older
> > (v1) patches to v3. So please ignore v3.
> >
> > Since the kernel cmdline accepts options like
> > "aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
> > also support it.
> >
> > Here is the previous series (v2):
> >   https://lore.kernel.org/lkml/162157886618.78209.11141970623539574861.stgit@devnote2/
> >
> > In this version, I rebased on top of the latest linus tree and
> > add the build error fix [1/6](*) and a cleanup patch [6/6].
> >
> > (*) https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/
> >
> > Changes in v4
> >  [1/6]:
> >      - Added from separated thread. This is a fundamental fix.
> >  [6/6]:
> >      - New cleanup patch.
> >
> >
> > With this series, sub-keys and a value can co-exist under a parent key.
> > For example, following config is allowed.
> >
> >  foo = value1
> >  foo.bar = value2
> >
> > Note, since there is no syntax to put a raw value directly under a
> > structured key, you have to define it outside of the brace. For example,
> >
> >  foo {
> >      bar = value1
> >      bar {
> >          baz = value2
> >          qux = value3
> >      }
> >  }
> >
> > Also, the order of the value node under a key is fixed. If there
> > are a value and subkeys, the value is always the first child node
> > of the key. Thus if user specifies subkeys first, e.g.
> >
> >  foo.bar = value1
> >  foo = value2
> >
> > In the program (and /proc/bootconfig), it will be shown as below
> >
> >  foo = value2
> >  foo.bar = value1
> >
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (6):
> >       tools/bootconfig: Fix a build error accroding to undefined fallthrough
> >       bootconfig: Change array value to use child node
> >       bootconfig: Support mixing a value and subkeys under a key
> >       tools/bootconfig: Support mixed value and subkey test cases
> >       docs: bootconfig: Update for mixing value and subkeys
> >       bootconfig: Share the checksum function with tools
> >
> >
> >  Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
> >  fs/proc/bootconfig.c                               |    2 -
> >  include/linux/bootconfig.h                         |   58 ++++++++++++++-
> >  init/main.c                                        |   12 ---
> >  lib/bootconfig.c                                   |   76 +++++++++++++++-----
> >  tools/bootconfig/include/linux/bootconfig.h        |    4 +
> >  tools/bootconfig/main.c                            |   62 +++++++++++-----
> >  tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
> >  tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
> >  tools/bootconfig/samples/bad-override.bconf        |    3 -
> >  tools/bootconfig/samples/bad-override2.bconf       |    3 -
> >  tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
> >  tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
> >  tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
> >  tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
> >  tools/bootconfig/samples/good-mixed-override.bconf |    4 +
> >  16 files changed, 205 insertions(+), 71 deletions(-)
> >  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-override.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> 
> Thanks for the updated patches.
> I tested the v4 changes on an Android virtual device(Cuttlefish) with
> a variety of
> parameters that included array values. I checked the output in /proc/bootconfig
> before and after the changes.
> I added a new parameter that failed before these changes and it worked great!
> Added 'androidboot.hardware=cutf_cvm ' which worked with
> 'androidboot.hardware.gralloc = "minigbm"' that was already present.
> 
> These changes really help Android simplify some boot configuration
> processes. We will be using it ASAP.


Thanks Devin.

Hi Steve, can you also pick this series? (except [1/6], which you've already merged)

Thank you,

> 
> So,
> 
> Tested-by: Devin Moore <devinmoore@google.com>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 14:48   ` Masami Hiramatsu
@ 2021-06-10 16:26     ` Steven Rostedt
  2021-06-10 16:38       ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2021-06-10 16:26 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Devin Moore, LKML, Ingo Molnar

On Thu, 10 Jun 2021 23:48:09 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Steve, can you also pick this series? (except [1/6], which you've already merged)

I may have already done so, just haven't pushed it yet. I'm using a
separate branch for bootconfig tools.

I need to set up a test suite that tests them.

I should set up a ktest config file that reads each of the example
bootconfigs, and boots the kernel, and checks that each of them took
affect without doing any other side effects.

I'll work to create a ktest config sample to test them, and send them
out.

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 16:26     ` Steven Rostedt
@ 2021-06-10 16:38       ` Steven Rostedt
  2021-06-10 16:48         ` Steven Rostedt
  2021-06-10 23:56         ` Masami Hiramatsu
  0 siblings, 2 replies; 17+ messages in thread
From: Steven Rostedt @ 2021-06-10 16:38 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Devin Moore, LKML, Ingo Molnar

On Thu, 10 Jun 2021 12:26:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I should set up a ktest config file that reads each of the example
> bootconfigs, and boots the kernel, and checks that each of them took
> affect without doing any other side effects.
> 
> I'll work to create a ktest config sample to test them, and send them
> out.

The sample configs seem to be just that, samples, and not something
that I can just load.

Do you have a set of bootconfig scripts I could use to load and boot
the kernel and make sure that they did what they intended to do?

Then I can make a series of ktest tests to test each one.

In the mean time, I'll just push out the branch to linux-next.

Thanks!

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 16:38       ` Steven Rostedt
@ 2021-06-10 16:48         ` Steven Rostedt
  2021-06-10 23:56         ` Masami Hiramatsu
  1 sibling, 0 replies; 17+ messages in thread
From: Steven Rostedt @ 2021-06-10 16:48 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Devin Moore, LKML, Ingo Molnar

On Thu, 10 Jun 2021 12:38:52 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> In the mean time, I'll just push out the branch to linux-next.

So much for keeping this separate. I just tried to merge it and it
gives me conflicts with the update I pulled in my other branch.

I guess it's fine to just add these on top of my other changes.

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 16:38       ` Steven Rostedt
  2021-06-10 16:48         ` Steven Rostedt
@ 2021-06-10 23:56         ` Masami Hiramatsu
  2021-06-16 19:17           ` Steven Rostedt
  2021-06-16 21:21           ` Steven Rostedt
  1 sibling, 2 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-10 23:56 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Devin Moore, LKML, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]

On Thu, 10 Jun 2021 12:38:52 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 10 Jun 2021 12:26:07 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > I should set up a ktest config file that reads each of the example
> > bootconfigs, and boots the kernel, and checks that each of them took
> > affect without doing any other side effects.

Thanks for adding bootconfig to the ktest!

> > 
> > I'll work to create a ktest config sample to test them, and send them
> > out.
> 
> The sample configs seem to be just that, samples, and not something
> that I can just load.
> 
> Do you have a set of bootconfig scripts I could use to load and boot
> the kernel and make sure that they did what they intended to do?

Yes, I have some example scripts. But maybe smaller example will be
better for test?
Let me break those example so that you can easily test that.

Also, you can find some syntax examples under tools/bootconfig/samples/.
(But I think those are already tested by bootconfig command anyway)

> 
> Then I can make a series of ktest tests to test each one.
> 
> In the mean time, I'll just push out the branch to linux-next.

Thank you!

> 
> Thanks!
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[-- Attachment #2: boottrace.bconf --]
[-- Type: application/octet-stream, Size: 1294 bytes --]

ftrace.event {
	task.task_newtask {
		filter = "pid < 128"
		enable
	}
	kprobes.vfs_read {
              probes = "vfs_read $arg1 $arg2"
              filter = "common_pid < 200"
              enable
      }
      synthetic.initcall_latency {
              fields = "unsigned long func", "u64 lat"
              actions = "hist:keys=func.sym,lat:vals=lat:sort=lat"
      }
      initcall.initcall_start {
              actions = "hist:keys=func:ts0=common_timestamp.usecs"
      }
      initcall.initcall_finish {
              actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)"
      }
}

ftrace.instance {
      foo {
              tracer = "function"
              ftrace.filters = "user_*"
              cpumask = 1
              options = nosym-addr
              buffer_size = 512KB
              trace_clock = mono
		event.signal.signal_deliver.actions=snapshot
      }
      bar {
              tracer = "function"
              ftrace.filters = "kernel_*"
	      cpumask = 2
             trace_clock = x86-tsc
      }
}

ftrace.alloc_snapshot

kernel {
      trace_options = sym-addr
      trace_events = "initcall:*"
#      tp_printk
      trace_buf_size = 1M
      ftrace = function
      ftrace_filter = "vfs*"
}

[-- Attachment #3: tracing.bconf --]
[-- Type: application/octet-stream, Size: 1129 bytes --]

ftrace {
	tracer = function;
	options = event-fork, sym-addr, stacktrace;
	buffer_size = 1M;
	alloc_snapshot;
	trace_clock = global;
	events = "task:task_newtask", "initcall:*";
	event.sched.sched_process_exec {
		filter = "pid < 128";
	}
	event.kprobes.myevent {
		probes = "vfs_read $arg1 $arg2", "vfs_write $arg1 $arg2"
	}
	event.kprobes.myevent2 {
		probes = "initrd_load";
	}
	event.kprobes.enable
	instance.bar {
		event.synthetic.initcall_latency {
			fields = "unsigned long func", "u64 lat";
			actions.hist {
				keys = func.sym, lat;
				vals = lat;
				sort = lat;
			}
		}
		event.initcall {
			initcall_start.actions.hist {
				keys = func;
				ts0 = "common_timestamp.usecs";
			}
			initcall_finish.actions.hist {
				keys = "func";
				lat = "common_timestamp.usecs-$ts0";
				onmatch = "initcall.initcall_start",
					  "initcall_latency(func,$lat)";
			}
		}
	}
	instance.foo {
		tracer = function-graph;
		tracing_on = false;
		event.workqueue.workqueue_start.actions = "tracing_on";
		event.workqueue.workqueue_end.actions = "tracing_off";
	};
}
kernel {
	tp_printk;
	dump_on_oops = 2
	traceoff_on_warning
}

[-- Attachment #4: functiongraph.bconf --]
[-- Type: application/octet-stream, Size: 403 bytes --]

ftrace {
      tracing_on = 0  # off by default
      tracer = function_graph
      event.kprobes {
                start_event {
                        probes = "pci_proc_init"
                        actions = "traceon"
                }
                end_event {
                        probes = "pci_proc_init%return"
                        actions = "traceoff"
                }
        }
  }


[-- Attachment #5: ftrace.bconf --]
[-- Type: application/octet-stream, Size: 1526 bytes --]

#!/bin/sh

ftrace {
	options = "sym-addr", "context-info"
	buffer_size = 1MB
}

ftrace.event {
	# Sample1: Make a histogram for initcall functions
	synthetic.initcall_latency {
                fields = "unsigned long func", "u64 lat"
		hist {
			keys = func.sym, lat
			vals = lat
			sort = lat
		}
	}
	initcall.initcall_start.hist {
		keys = func
		var.ts0 = common_timestamp.usecs
	}
	initcall.initcall_finish.hist {
		keys = func
		var.lat = common_timestamp.usecs - $ts0
		onmatch {
			event = initcall.initcall_start
			action = "initcall_latency(func, $lat)"
		}
	}

	# Sample2: kmalloc() tracing in read(2) syscall
	syscalls.sys_enter_read.enable_event {
		event = kmem.kmalloc
		count = 1
	}
	syscalls.sys_exit_read.disable_event {
		event = kmem.kmalloc
	}

	# Sample3: Stacktrace at the event
	kmem.kmalloc.stacktrace {
		count = 5
		filter = 'bytes_req >= 65536'
	}

	# Sample4: Take a snapshot
	block.block_unplug.snapshot {
		count = 1
		filter = nr_rq > 1
	}

	# Sample5: Trace-on/off
	block.block_plug.traceon {
		filter = nr_rq > 1
	}
	block.block_unplug.traceoff {
		filter = nr_rq > 1
	}

        # Sample6: onmax
	sched.sched_waking {
		enable
		hist {
			keys = pid
			ts1 = common_timestamp.usecs
			filter = 'comm == "cyclictest"'
		}
	}
	sched.sched_switch {
		enable
		hist {
			keys = next_pid
			var.wakeup_lat = common_timestamp.usecs - $ts1
			onmax {
				var = wakeup_lat
				action = "save(next_prio,next_comm,prev_pid,prev_prio,prev_comm)"
			}
			filter = 'next_comm == "cyclictest"'
		}
	}
}


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 23:56         ` Masami Hiramatsu
@ 2021-06-16 19:17           ` Steven Rostedt
  2021-06-17  1:27             ` Masami Hiramatsu
  2021-06-16 21:21           ` Steven Rostedt
  1 sibling, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2021-06-16 19:17 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Devin Moore, LKML, Ingo Molnar

On Fri, 11 Jun 2021 08:56:35 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> kernel {
>       trace_options = sym-addr
>       trace_events = "initcall:*"
> #      tp_printk
>       trace_buf_size = 1M
>       ftrace = function
>       ftrace_filter = "vfs*"
> }

I'm creating a ktest test for boot up on bootconfigs and writing a verifier
for each of the configs you sent me. On the first config I looked at
"boottrace.bconf", I found a bug.

The above should be:

	trace_event = "initcall:*"

and not "trace_events" as "trace_events" is not a boot option ;-)

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-10 23:56         ` Masami Hiramatsu
  2021-06-16 19:17           ` Steven Rostedt
@ 2021-06-16 21:21           ` Steven Rostedt
  2021-06-17  1:31             ` Masami Hiramatsu
  1 sibling, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2021-06-16 21:21 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Devin Moore, LKML, Ingo Molnar

On Fri, 11 Jun 2021 08:56:35 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> > Do you have a set of bootconfig scripts I could use to load and boot
> > the kernel and make sure that they did what they intended to do?  
> 
> Yes, I have some example scripts. But maybe smaller example will be
> better for test?
> Let me break those example so that you can easily test that.

Do these scripts need updates? Because I was able to get two of them
working (boottrace.bconf and functiongraph.bconf sorta) but two seem to
require more features from the kernel (ftrace.bconf and tracing.bconf).

Anyway, I'll finish up the ktest script and post something soon.

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-16 19:17           ` Steven Rostedt
@ 2021-06-17  1:27             ` Masami Hiramatsu
  0 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-17  1:27 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Devin Moore, LKML, Ingo Molnar

On Wed, 16 Jun 2021 15:17:01 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 11 Jun 2021 08:56:35 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > kernel {
> >       trace_options = sym-addr
> >       trace_events = "initcall:*"
> > #      tp_printk
> >       trace_buf_size = 1M
> >       ftrace = function
> >       ftrace_filter = "vfs*"
> > }
> 
> I'm creating a ktest test for boot up on bootconfigs and writing a verifier
> for each of the configs you sent me. On the first config I looked at
> "boottrace.bconf", I found a bug.
> 
> The above should be:
> 
> 	trace_event = "initcall:*"
> 
> and not "trace_events" as "trace_events" is not a boot option ;-)

Oops, thanks!

BTW, I'm thinking to introduce more basic tests for kernel cmdlie.
For example, checking the kernel.* and init.* are merged correctly
in the /proc/cmdline.

But yes, this needs to use ktest because it sets the bootconfig
to initrd and reboots with it for testing.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key
  2021-06-16 21:21           ` Steven Rostedt
@ 2021-06-17  1:31             ` Masami Hiramatsu
  0 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2021-06-17  1:31 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Devin Moore, LKML, Ingo Molnar

On Wed, 16 Jun 2021 17:21:47 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 11 Jun 2021 08:56:35 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > Do you have a set of bootconfig scripts I could use to load and boot
> > > the kernel and make sure that they did what they intended to do?  
> > 
> > Yes, I have some example scripts. But maybe smaller example will be
> > better for test?
> > Let me break those example so that you can easily test that.
> 
> Do these scripts need updates? Because I was able to get two of them
> working (boottrace.bconf and functiongraph.bconf sorta) but two seem to
> require more features from the kernel (ftrace.bconf and tracing.bconf).

Oops again. Yes those include some extended (experimental) syntax.
please ignore it until I updated boot-time tracing.

> 
> Anyway, I'll finish up the ktest script and post something soon.

Thank you!

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-06-17  1:31 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02  8:18 [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Masami Hiramatsu
2021-06-02  8:18 ` [PATCH v4 1/6] tools/bootconfig: Fix a build error accroding to undefined fallthrough Masami Hiramatsu
2021-06-02  8:18 ` [PATCH v4 2/6] bootconfig: Change array value to use child node Masami Hiramatsu
2021-06-02  8:19 ` [PATCH v4 3/6] bootconfig: Support mixing a value and subkeys under a key Masami Hiramatsu
2021-06-02  8:19 ` [PATCH v4 4/6] tools/bootconfig: Support mixed value and subkey test cases Masami Hiramatsu
2021-06-02  8:19 ` [PATCH v4 5/6] docs: bootconfig: Update for mixing value and subkeys Masami Hiramatsu
2021-06-02  8:19 ` [PATCH v4 6/6] bootconfig: Share the checksum function with tools Masami Hiramatsu
2021-06-02 23:02 ` [PATCH v4 0/6] bootconfig: Add mixed subkeys and value under the same key Devin Moore
2021-06-10 14:48   ` Masami Hiramatsu
2021-06-10 16:26     ` Steven Rostedt
2021-06-10 16:38       ` Steven Rostedt
2021-06-10 16:48         ` Steven Rostedt
2021-06-10 23:56         ` Masami Hiramatsu
2021-06-16 19:17           ` Steven Rostedt
2021-06-17  1:27             ` Masami Hiramatsu
2021-06-16 21:21           ` Steven Rostedt
2021-06-17  1:31             ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).