[PATCH 0/6] Shrinking DT memory usage

[PATCH 0/6] Shrinking DT memory usage

[Rob Herring]

On kernels with a minimal config and a RAM target in the 100s of KB, DT 
is quite a hog of runtime memory usage. How much is dependent on how many
nodes and properties in the DT which have a corresponding struct device_node
and struct property in the kernel. Just skipping disabled nodes saves a 
lot by not creating the device_nodes in the first place[1], but there's 
more low hanging fruit by making some of the fields in struct property and
struct device_node optional. With the changes here, the memory usage goes
from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively 
small DT.

The majority of the diffstat here is just moving all the kobject/sysfs 
related code to its own file so we can avoid adding a bunch of ifdefs.

There's more drastic approaches we could take such as doing the 
unflattening at build time and storing the bulk of the unflattened tree 
as const data. Grant also has some ideas on storing properties as ids 
instead. He's explained it to me, but I still don't understand it.

Rob

[1] https://patchwork.kernel.org/patch/9983203/

Rob Herring (6):
  of: fix missing kobject init for !SYSFS && OF_DYNAMIC config
  of: remove struct property.unique_id for FDT
  of: make struct property _flags field configurable
  of: move kobj_to_device_node() into dynamic.c
  of: wrap accesses to device_node kobject
  of: make kobject and bin_attribute support configurable

 drivers/base/core.c     |   2 +-
 drivers/of/Kconfig      |   4 ++
 drivers/of/Makefile     |   1 +
 drivers/of/base.c       | 133 ---------------------------------------
 drivers/of/dynamic.c    |  27 ++------
 drivers/of/kobj.c       | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/of_private.h |  29 +++++++--
 include/linux/of.h      |  28 +++++----
 8 files changed, 215 insertions(+), 173 deletions(-)
 create mode 100644 drivers/of/kobj.c

-- 
2.11.0

[PATCH 0/6] Shrinking DT memory usage

[Rob Herring]

On kernels with a minimal config and a RAM target in the 100s of KB, DT 
is quite a hog of runtime memory usage. How much is dependent on how many
nodes and properties in the DT which have a corresponding struct device_node
and struct property in the kernel. Just skipping disabled nodes saves a 
lot by not creating the device_nodes in the first place[1], but there's 
more low hanging fruit by making some of the fields in struct property and
struct device_node optional. With the changes here, the memory usage goes
from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively 
small DT.

The majority of the diffstat here is just moving all the kobject/sysfs 
related code to its own file so we can avoid adding a bunch of ifdefs.

There's more drastic approaches we could take such as doing the 
unflattening at build time and storing the bulk of the unflattened tree 
as const data. Grant also has some ideas on storing properties as ids 
instead. He's explained it to me, but I still don't understand it.

Rob

[1] https://patchwork.kernel.org/patch/9983203/

Rob Herring (6):
  of: fix missing kobject init for !SYSFS && OF_DYNAMIC config
  of: remove struct property.unique_id for FDT
  of: make struct property _flags field configurable
  of: move kobj_to_device_node() into dynamic.c
  of: wrap accesses to device_node kobject
  of: make kobject and bin_attribute support configurable

 drivers/base/core.c     |   2 +-
 drivers/of/Kconfig      |   4 ++
 drivers/of/Makefile     |   1 +
 drivers/of/base.c       | 133 ---------------------------------------
 drivers/of/dynamic.c    |  27 ++------
 drivers/of/kobj.c       | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/of_private.h |  29 +++++++--
 include/linux/of.h      |  28 +++++----
 8 files changed, 215 insertions(+), 173 deletions(-)
 create mode 100644 drivers/of/kobj.c

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] of: fix missing kobject init for !SYSFS && OF_DYNAMIC config

[Rob Herring]

The ref counting is broken for OF_DYNAMIC when sysfs is disabled because
the kobject initialization is skipped. Only the properties
add/remove/update should be skipped for !SYSFS config.

Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/of/base.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 260d33c0f26c..9e9bd17121fb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -169,9 +169,6 @@ int __of_attach_node_sysfs(struct device_node *np)
 	struct property *pp;
 	int rc;
 
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return 0;
-
 	if (!of_kset)
 		return 0;
 
-- 
2.11.0

[PATCH 1/6] of: fix missing kobject init for !SYSFS && OF_DYNAMIC config

[Rob Herring]

The ref counting is broken for OF_DYNAMIC when sysfs is disabled because
the kobject initialization is skipped. Only the properties
add/remove/update should be skipped for !SYSFS config.

Cc: Nicolas Pitre <nico-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/of/base.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 260d33c0f26c..9e9bd17121fb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -169,9 +169,6 @@ int __of_attach_node_sysfs(struct device_node *np)
 	struct property *pp;
 	int rc;
 
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return 0;
-
 	if (!of_kset)
 		return 0;
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] of: remove struct property.unique_id for FDT

[Rob Herring]

Only Sparc uses unique_id, so remove it for FDT builds and shrink struct
property a bit making the unflattened DT less of a memory hog.

Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 include/linux/of.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index cfc34117fc92..8f9e96752837 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -38,7 +38,9 @@ struct property {
 	void	*value;
 	struct property *next;
 	unsigned long _flags;
+#if defined(CONFIG_OF_PROMTREE)
 	unsigned int unique_id;
+#endif
 	struct bin_attribute attr;
 };
 
-- 
2.11.0

[PATCH 3/6] of: make struct property _flags field configurable

[Rob Herring]

Only Sparc and CONFIG_OF_DYNAMIC use the struct property._flags field,
so make it conditional shrinking struct property a bit.

Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 include/linux/of.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 8f9e96752837..7eb94b7fbcf3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -37,7 +37,9 @@ struct property {
 	int	length;
 	void	*value;
 	struct property *next;
+#if defined(CONFIG_OF_DYNAMIC) || defined(CONFIG_SPARC)
 	unsigned long _flags;
+#endif
 #if defined(CONFIG_OF_PROMTREE)
 	unsigned int unique_id;
 #endif
@@ -205,6 +207,7 @@ static inline void of_node_clear_flag(struct device_node *n, unsigned long flag)
 	clear_bit(flag, &n->_flags);
 }
 
+#if defined(CONFIG_OF_DYNAMIC) || defined(CONFIG_SPARC)
 static inline int of_property_check_flag(struct property *p, unsigned long flag)
 {
 	return test_bit(flag, &p->_flags);
@@ -219,6 +222,7 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
 {
 	clear_bit(flag, &p->_flags);
 }
+#endif
 
 extern struct device_node *__of_find_all_nodes(struct device_node *prev);
 extern struct device_node *of_find_all_nodes(struct device_node *prev);
-- 
2.11.0

[PATCH 4/6] of: move kobj_to_device_node() into dynamic.c

[Rob Herring]

The only user of kobj_to_device_node() is in dynamic.c, so move it
there. This avoids having to make it conditional once kobject is
configurable.

Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/of/dynamic.c    | 5 +++++
 drivers/of/of_private.h | 6 ------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 301b6db2b48d..9d6ba18c529f 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -16,6 +16,11 @@
 
 #include "of_private.h"
 
+static struct device_node *kobj_to_device_node(struct kobject *kobj)
+{
+	return container_of(kobj, struct device_node, kobj);
+}
+
 /**
  * of_node_get() - Increment refcount of a node
  * @node:	Node to inc refcount, NULL is supported to simplify writing of
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 3ae12ffbf547..cc86a974f35f 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -35,12 +35,6 @@ extern struct mutex of_mutex;
 extern struct list_head aliases_lookup;
 extern struct kset *of_kset;
 
-
-static inline struct device_node *kobj_to_device_node(struct kobject *kobj)
-{
-	return container_of(kobj, struct device_node, kobj);
-}
-
 #if defined(CONFIG_OF_DYNAMIC)
 extern int of_property_notify(int action, struct device_node *np,
 			      struct property *prop, struct property *old_prop);
-- 
2.11.0

[PATCH 5/6] of: wrap accesses to device_node kobject

[Rob Herring]

In preparation to make kobject element in struct device_node optional,
provide and use a macro to return the kobject pointer. The only user
outside the DT core is the driver core.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/core.c | 2 +-
 include/linux/of.h  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 12ebd055724c..c07b47059538 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1571,7 +1571,7 @@ static int device_add_class_symlinks(struct device *dev)
 	int error;
 
 	if (of_node) {
-		error = sysfs_create_link(&dev->kobj, &of_node->kobj,"of_node");
+		error = sysfs_create_link(&dev->kobj, of_node_kobj(of_node), "of_node");
 		if (error)
 			dev_warn(dev, "Error %d creating of_node link\n",error);
 		/* An error here doesn't warrant bringing down the device */
diff --git a/include/linux/of.h b/include/linux/of.h
index 7eb94b7fbcf3..2d685e769409 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -111,6 +111,8 @@ static inline void of_node_init(struct device_node *node)
 	node->fwnode.ops = &of_fwnode_ops;
 }
 
+#define of_node_kobj(n) (&(n)->kobj)
+
 /* true when node is initialized */
 static inline int of_node_is_initialized(struct device_node *node)
 {
-- 
2.11.0

[PATCH 6/6] of: make kobject and bin_attribute support configurable

[Rob Herring]

Having device_nodes be kobjects is only needed if sysfs or OF_DYNAMIC is
enabled. Otherwise, having a kobject in struct device_node is
unnecessary bloat in minimal kernel configurations.

Likewise, bin_attribute is only needed in struct property when sysfs is
enabled, so we can make it configurable too.

Cc: Nicolas Pitre <nico@linaro.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/of/Kconfig      |   4 ++
 drivers/of/Makefile     |   1 +
 drivers/of/base.c       | 130 --------------------------------------
 drivers/of/dynamic.c    |  22 -------
 drivers/of/kobj.c       | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/of_private.h |  23 +++++++
 include/linux/of.h      |  22 +++----
 7 files changed, 202 insertions(+), 164 deletions(-)
 create mode 100644 drivers/of/kobj.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index ba7b034b2b91..ad9a9578f9c4 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -46,10 +46,14 @@ config OF_EARLY_FLATTREE
 config OF_PROMTREE
 	bool
 
+config OF_KOBJ
+	def_bool SYSFS
+
 # Hardly any platforms need this.  It is safe to select, but only do so if you
 # need it.
 config OF_DYNAMIC
 	bool "Support for dynamic device trees" if OF_UNITTEST
+	select OF_KOBJ
 	help
 	  On some platforms, the device tree can be manipulated at runtime.
 	  While this option is selected automatically on such platforms, you
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 97dc01c81438..83d61a7c5e82 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -1,4 +1,5 @@
 obj-y = base.o device.o platform.o property.o
+obj-$(CONFIG_OF_KOBJ) += kobj.o
 obj-$(CONFIG_OF_DYNAMIC) += dynamic.o
 obj-$(CONFIG_OF_FLATTREE) += fdt.o
 obj-$(CONFIG_OF_EARLY_FLATTREE) += fdt_address.o
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9e9bd17121fb..b98f3adffbb0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -95,105 +95,6 @@ int __weak of_node_to_nid(struct device_node *np)
 }
 #endif
 
-#ifndef CONFIG_OF_DYNAMIC
-static void of_node_release(struct kobject *kobj)
-{
-	/* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
-}
-#endif /* CONFIG_OF_DYNAMIC */
-
-struct kobj_type of_node_ktype = {
-	.release = of_node_release,
-};
-
-static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
-				struct bin_attribute *bin_attr, char *buf,
-				loff_t offset, size_t count)
-{
-	struct property *pp = container_of(bin_attr, struct property, attr);
-	return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
-}
-
-/* always return newly allocated name, caller must free after use */
-static const char *safe_name(struct kobject *kobj, const char *orig_name)
-{
-	const char *name = orig_name;
-	struct kernfs_node *kn;
-	int i = 0;
-
-	/* don't be a hero. After 16 tries give up */
-	while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) {
-		sysfs_put(kn);
-		if (name != orig_name)
-			kfree(name);
-		name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
-	}
-
-	if (name == orig_name) {
-		name = kstrdup(orig_name, GFP_KERNEL);
-	} else {
-		pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
-			kobject_name(kobj), name);
-	}
-	return name;
-}
-
-int __of_add_property_sysfs(struct device_node *np, struct property *pp)
-{
-	int rc;
-
-	/* Important: Don't leak passwords */
-	bool secure = strncmp(pp->name, "security-", 9) == 0;
-
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return 0;
-
-	if (!of_kset || !of_node_is_attached(np))
-		return 0;
-
-	sysfs_bin_attr_init(&pp->attr);
-	pp->attr.attr.name = safe_name(&np->kobj, pp->name);
-	pp->attr.attr.mode = secure ? 0400 : 0444;
-	pp->attr.size = secure ? 0 : pp->length;
-	pp->attr.read = of_node_property_read;
-
-	rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
-	WARN(rc, "error adding attribute %s to node %pOF\n", pp->name, np);
-	return rc;
-}
-
-int __of_attach_node_sysfs(struct device_node *np)
-{
-	const char *name;
-	struct kobject *parent;
-	struct property *pp;
-	int rc;
-
-	if (!of_kset)
-		return 0;
-
-	np->kobj.kset = of_kset;
-	if (!np->parent) {
-		/* Nodes without parents are new top level trees */
-		name = safe_name(&of_kset->kobj, "base");
-		parent = NULL;
-	} else {
-		name = safe_name(&np->parent->kobj, kbasename(np->full_name));
-		parent = &np->parent->kobj;
-	}
-	if (!name)
-		return -ENOMEM;
-	rc = kobject_add(&np->kobj, parent, "%s", name);
-	kfree(name);
-	if (rc)
-		return rc;
-
-	for_each_property_of_node(np, pp)
-		__of_add_property_sysfs(np, pp);
-
-	return 0;
-}
-
 void __init of_core_init(void)
 {
 	struct device_node *np;
@@ -1501,22 +1402,6 @@ int __of_remove_property(struct device_node *np, struct property *prop)
 	return 0;
 }
 
-void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop)
-{
-	sysfs_remove_bin_file(&np->kobj, &prop->attr);
-	kfree(prop->attr.attr.name);
-}
-
-void __of_remove_property_sysfs(struct device_node *np, struct property *prop)
-{
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return;
-
-	/* at early boot, bail here and defer setup to of_init() */
-	if (of_kset && of_node_is_attached(np))
-		__of_sysfs_remove_bin_file(np, prop);
-}
-
 /**
  * of_remove_property - Remove a property from a node.
  *
@@ -1576,21 +1461,6 @@ int __of_update_property(struct device_node *np, struct property *newprop,
 	return 0;
 }
 
-void __of_update_property_sysfs(struct device_node *np, struct property *newprop,
-		struct property *oldprop)
-{
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return;
-
-	/* At early boot, bail out and defer setup to of_init() */
-	if (!of_kset)
-		return;
-
-	if (oldprop)
-		__of_sysfs_remove_bin_file(np, oldprop);
-	__of_add_property_sysfs(np, newprop);
-}
-
 /*
  * of_update_property - Update a property in a node, if the property does
  * not exist, add it.
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 9d6ba18c529f..39e8cf731764 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -48,28 +48,6 @@ void of_node_put(struct device_node *node)
 }
 EXPORT_SYMBOL(of_node_put);
 
-void __of_detach_node_sysfs(struct device_node *np)
-{
-	struct property *pp;
-
-	if (!IS_ENABLED(CONFIG_SYSFS))
-		return;
-
-	BUG_ON(!of_node_is_initialized(np));
-	if (!of_kset)
-		return;
-
-	/* only remove properties if on sysfs */
-	if (of_node_is_attached(np)) {
-		for_each_property_of_node(np, pp)
-			__of_sysfs_remove_bin_file(np, pp);
-		kobject_del(&np->kobj);
-	}
-
-	/* finally remove the kobj_init ref */
-	of_node_put(np);
-}
-
 static BLOCKING_NOTIFIER_HEAD(of_reconfig_chain);
 
 int of_reconfig_notifier_register(struct notifier_block *nb)
diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
new file mode 100644
index 000000000000..250fc7bb550f
--- /dev/null
+++ b/drivers/of/kobj.c
@@ -0,0 +1,164 @@
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include "of_private.h"
+
+/* true when node is initialized */
+static int of_node_is_initialized(struct device_node *node)
+{
+	return node && node->kobj.state_initialized;
+}
+
+/* true when node is attached (i.e. present on sysfs) */
+int of_node_is_attached(struct device_node *node)
+{
+	return node && node->kobj.state_in_sysfs;
+}
+
+
+#ifndef CONFIG_OF_DYNAMIC
+static void of_node_release(struct kobject *kobj)
+{
+	/* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
+}
+#endif /* CONFIG_OF_DYNAMIC */
+
+struct kobj_type of_node_ktype = {
+	.release = of_node_release,
+};
+
+static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t offset, size_t count)
+{
+	struct property *pp = container_of(bin_attr, struct property, attr);
+	return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
+}
+
+/* always return newly allocated name, caller must free after use */
+static const char *safe_name(struct kobject *kobj, const char *orig_name)
+{
+	const char *name = orig_name;
+	struct kernfs_node *kn;
+	int i = 0;
+
+	/* don't be a hero. After 16 tries give up */
+	while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) {
+		sysfs_put(kn);
+		if (name != orig_name)
+			kfree(name);
+		name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
+	}
+
+	if (name == orig_name) {
+		name = kstrdup(orig_name, GFP_KERNEL);
+	} else {
+		pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
+			kobject_name(kobj), name);
+	}
+	return name;
+}
+
+int __of_add_property_sysfs(struct device_node *np, struct property *pp)
+{
+	int rc;
+
+	/* Important: Don't leak passwords */
+	bool secure = strncmp(pp->name, "security-", 9) == 0;
+
+	if (!IS_ENABLED(CONFIG_SYSFS))
+		return 0;
+
+	if (!of_kset || !of_node_is_attached(np))
+		return 0;
+
+	sysfs_bin_attr_init(&pp->attr);
+	pp->attr.attr.name = safe_name(&np->kobj, pp->name);
+	pp->attr.attr.mode = secure ? 0400 : 0444;
+	pp->attr.size = secure ? 0 : pp->length;
+	pp->attr.read = of_node_property_read;
+
+	rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
+	WARN(rc, "error adding attribute %s to node %pOF\n", pp->name, np);
+	return rc;
+}
+
+void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop)
+{
+	if (!IS_ENABLED(CONFIG_SYSFS))
+		return;
+
+	sysfs_remove_bin_file(&np->kobj, &prop->attr);
+	kfree(prop->attr.attr.name);
+}
+
+void __of_remove_property_sysfs(struct device_node *np, struct property *prop)
+{
+	/* at early boot, bail here and defer setup to of_init() */
+	if (of_kset && of_node_is_attached(np))
+		__of_sysfs_remove_bin_file(np, prop);
+}
+
+void __of_update_property_sysfs(struct device_node *np, struct property *newprop,
+		struct property *oldprop)
+{
+	/* At early boot, bail out and defer setup to of_init() */
+	if (!of_kset)
+		return;
+
+	if (oldprop)
+		__of_sysfs_remove_bin_file(np, oldprop);
+	__of_add_property_sysfs(np, newprop);
+}
+
+int __of_attach_node_sysfs(struct device_node *np)
+{
+	const char *name;
+	struct kobject *parent;
+	struct property *pp;
+	int rc;
+
+	if (!of_kset)
+		return 0;
+
+	np->kobj.kset = of_kset;
+	if (!np->parent) {
+		/* Nodes without parents are new top level trees */
+		name = safe_name(&of_kset->kobj, "base");
+		parent = NULL;
+	} else {
+		name = safe_name(&np->parent->kobj, kbasename(np->full_name));
+		parent = &np->parent->kobj;
+	}
+	if (!name)
+		return -ENOMEM;
+	rc = kobject_add(&np->kobj, parent, "%s", name);
+	kfree(name);
+	if (rc)
+		return rc;
+
+	for_each_property_of_node(np, pp)
+		__of_add_property_sysfs(np, pp);
+
+	return 0;
+}
+
+void __of_detach_node_sysfs(struct device_node *np)
+{
+	struct property *pp;
+
+	BUG_ON(!of_node_is_initialized(np));
+	if (!of_kset)
+		return;
+
+	/* only remove properties if on sysfs */
+	if (of_node_is_attached(np)) {
+		for_each_property_of_node(np, pp)
+			__of_sysfs_remove_bin_file(np, pp);
+		kobject_del(&np->kobj);
+	}
+
+	/* finally remove the kobj_init ref */
+	of_node_put(np);
+}
+
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index cc86a974f35f..43df14f0cbce 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -49,6 +49,29 @@ static inline int of_property_notify(int action, struct device_node *np,
 }
 #endif /* CONFIG_OF_DYNAMIC */
 
+#if defined(CONFIG_OF_KOBJ)
+int of_node_is_attached(struct device_node *node);
+int __of_add_property_sysfs(struct device_node *np, struct property *pp);
+void __of_remove_property_sysfs(struct device_node *np, struct property *prop);
+void __of_update_property_sysfs(struct device_node *np, struct property *newprop,
+		struct property *oldprop);
+int __of_attach_node_sysfs(struct device_node *np);
+void __of_detach_node_sysfs(struct device_node *np);
+#else
+static inline int __of_add_property_sysfs(struct device_node *np, struct property *pp)
+{
+	return 0;
+}
+static inline void __of_remove_property_sysfs(struct device_node *np, struct property *prop) {}
+static inline void __of_update_property_sysfs(struct device_node *np,
+		struct property *newprop, struct property *oldprop) {}
+static inline int __of_attach_node_sysfs(struct device_node *np)
+{
+	return 0;
+}
+static inline void __of_detach_node_sysfs(struct device_node *np) {}
+#endif
+
 #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY)
 extern void __init unittest_unflatten_overlay_base(void);
 #else
diff --git a/include/linux/of.h b/include/linux/of.h
index 2d685e769409..7b0f17be7830 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -43,7 +43,9 @@ struct property {
 #if defined(CONFIG_OF_PROMTREE)
 	unsigned int unique_id;
 #endif
+#if defined(CONFIG_OF_KOBJ)
 	struct bin_attribute attr;
+#endif
 };
 
 #if defined(CONFIG_SPARC)
@@ -62,7 +64,9 @@ struct device_node {
 	struct	device_node *parent;
 	struct	device_node *child;
 	struct	device_node *sibling;
+#if defined(CONFIG_OF_KOBJ)
 	struct	kobject kobj;
+#endif
 	unsigned long _flags;
 	void	*data;
 #if defined(CONFIG_SPARC)
@@ -107,23 +111,17 @@ extern struct kobj_type of_node_ktype;
 extern const struct fwnode_operations of_fwnode_ops;
 static inline void of_node_init(struct device_node *node)
 {
+#if defined(CONFIG_OF_KOBJ)
 	kobject_init(&node->kobj, &of_node_ktype);
+#endif
 	node->fwnode.ops = &of_fwnode_ops;
 }
 
+#if defined(CONFIG_OF_KOBJ)
 #define of_node_kobj(n) (&(n)->kobj)
-
-/* true when node is initialized */
-static inline int of_node_is_initialized(struct device_node *node)
-{
-	return node && node->kobj.state_initialized;
-}
-
-/* true when node is attached (i.e. present on sysfs) */
-static inline int of_node_is_attached(struct device_node *node)
-{
-	return node && node->kobj.state_in_sysfs;
-}
+#else
+#define of_node_kobj(n) NULL
+#endif
 
 #ifdef CONFIG_OF_DYNAMIC
 extern struct device_node *of_node_get(struct device_node *node);
-- 
2.11.0

Re: [PATCH 5/6] of: wrap accesses to device_node kobject

[Greg Kroah-Hartman]

On Thu, Oct 05, 2017 at 02:44:21PM -0500, Rob Herring wrote:
> In preparation to make kobject element in struct device_node optional,
> provide and use a macro to return the kobject pointer. The only user
> outside the DT core is the driver core.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Nicolas Pitre <nico@linaro.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/core.c | 2 +-
>  include/linux/of.h  | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 5/6] of: wrap accesses to device_node kobject

[Greg Kroah-Hartman]

On Thu, Oct 05, 2017 at 02:44:21PM -0500, Rob Herring wrote:
> In preparation to make kobject element in struct device_node optional,
> provide and use a macro to return the kobject pointer. The only user
> outside the DT core is the driver core.
> 
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: Nicolas Pitre <nico-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/base/core.c | 2 +-
>  include/linux/of.h  | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] of: remove struct property.unique_id for FDT

[Grant Likely]

On Thu, Oct 5, 2017 at 8:44 PM, Rob Herring <robh@kernel.org> wrote:
> Only Sparc uses unique_id, so remove it for FDT builds and shrink struct
> property a bit making the unflattened DT less of a memory hog.

It can be even more constrained than that. I think the only user is
openpromfs so you could make it depend on CONFIG_SUN_OPENPROMFS, and
adjust the needed code in pdt.c.

Ideally, I'd like to be rid of unique_id entirely, but that's a much
more invasive patch set. Regardless:

Acked-by: Grant Likely <grant.likely@secretlab.ca>

(That goes for the entire series)

>
> Cc: Nicolas Pitre <nico@linaro.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  include/linux/of.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index cfc34117fc92..8f9e96752837 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -38,7 +38,9 @@ struct property {
>         void    *value;
>         struct property *next;
>         unsigned long _flags;
> +#if defined(CONFIG_OF_PROMTREE)
>         unsigned int unique_id;
> +#endif
>         struct bin_attribute attr;
>  };
>
> --
> 2.11.0
>

Re: [PATCH 0/6] Shrinking DT memory usage

[Grant Likely]

On Thu, Oct 5, 2017 at 8:44 PM, Rob Herring <robh@kernel.org> wrote:
> On kernels with a minimal config and a RAM target in the 100s of KB, DT
> is quite a hog of runtime memory usage. How much is dependent on how many
> nodes and properties in the DT which have a corresponding struct device_node
> and struct property in the kernel. Just skipping disabled nodes saves a
> lot by not creating the device_nodes in the first place[1], but there's
> more low hanging fruit by making some of the fields in struct property and
> struct device_node optional. With the changes here, the memory usage goes
> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively
> small DT.
>
> The majority of the diffstat here is just moving all the kobject/sysfs
> related code to its own file so we can avoid adding a bunch of ifdefs.
>
> There's more drastic approaches we could take such as doing the
> unflattening at build time and storing the bulk of the unflattened tree
> as const data. Grant also has some ideas on storing properties as ids
> instead. He's explained it to me, but I still don't understand it.

It was a two part idea. The first part is to start using numerical IDs
for property names instead of strings. The DTB format kind of already
does this because all the property names are stored in the string
table at the end of the dtb. The actual properties just reference an
offset into the string table. If we predefined ids for some property
names (reg, compatible, status, etc.), then we could have a variant of
the of_property_* API to find properties by numerical ID instead of by
strcmp().

The second idea is to eliminate struct property entirely and always
store a node's properties as a single DTB tagged list blob. It would
reduce the space required at the expense of processing time when
parsing the blob. The upshot is the property list could be left in a
flash-resident copy of the DTB.

Details need to be investigated, and it would be much more invasive
than what Rob has been able to do here. It may not be worth the
effort.

g.

Re: [PATCH 0/6] Shrinking DT memory usage

[Nicolas Pitre]

On Thu, 5 Oct 2017, Rob Herring wrote:

> On kernels with a minimal config and a RAM target in the 100s of KB, DT 
> is quite a hog of runtime memory usage. How much is dependent on how many
> nodes and properties in the DT which have a corresponding struct device_node
> and struct property in the kernel. Just skipping disabled nodes saves a 
> lot by not creating the device_nodes in the first place[1], but there's 
> more low hanging fruit by making some of the fields in struct property and
> struct device_node optional. With the changes here, the memory usage goes
> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively 
> small DT.

My test case went from 118072 bytes ddown to 21548 bytes with this 
series.

Tested-by: Nicolas Pitre <nico@linaro.org>


Nicolas

Re: [PATCH 0/6] Shrinking DT memory usage

[Rob Herring]

On Fri, Oct 6, 2017 at 4:30 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Thu, 5 Oct 2017, Rob Herring wrote:
>
>> On kernels with a minimal config and a RAM target in the 100s of KB, DT
>> is quite a hog of runtime memory usage. How much is dependent on how many
>> nodes and properties in the DT which have a corresponding struct device_node
>> and struct property in the kernel. Just skipping disabled nodes saves a
>> lot by not creating the device_nodes in the first place[1], but there's
>> more low hanging fruit by making some of the fields in struct property and
>> struct device_node optional. With the changes here, the memory usage goes
>> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively
>> small DT.
>
> My test case went from 118072 bytes ddown to 21548 bytes with this
> series.

Did that include the changes for FDT skipping status=disabled and to
stop storing the full path of every node? With those 2 alone you said
it dropped to 25K.

> Tested-by: Nicolas Pitre <nico@linaro.org>

Thanks.

Rob

Re: [PATCH 0/6] Shrinking DT memory usage

[Rob Herring]

On Fri, Oct 6, 2017 at 4:30 PM, Nicolas Pitre <nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Thu, 5 Oct 2017, Rob Herring wrote:
>
>> On kernels with a minimal config and a RAM target in the 100s of KB, DT
>> is quite a hog of runtime memory usage. How much is dependent on how many
>> nodes and properties in the DT which have a corresponding struct device_node
>> and struct property in the kernel. Just skipping disabled nodes saves a
>> lot by not creating the device_nodes in the first place[1], but there's
>> more low hanging fruit by making some of the fields in struct property and
>> struct device_node optional. With the changes here, the memory usage goes
>> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively
>> small DT.
>
> My test case went from 118072 bytes ddown to 21548 bytes with this
> series.

Did that include the changes for FDT skipping status=disabled and to
stop storing the full path of every node? With those 2 alone you said
it dropped to 25K.

> Tested-by: Nicolas Pitre <nico-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Thanks.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] Shrinking DT memory usage

[Nicolas Pitre]

On Fri, 6 Oct 2017, Rob Herring wrote:

> On Fri, Oct 6, 2017 at 4:30 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 5 Oct 2017, Rob Herring wrote:
> >
> >> On kernels with a minimal config and a RAM target in the 100s of KB, DT
> >> is quite a hog of runtime memory usage. How much is dependent on how many
> >> nodes and properties in the DT which have a corresponding struct device_node
> >> and struct property in the kernel. Just skipping disabled nodes saves a
> >> lot by not creating the device_nodes in the first place[1], but there's
> >> more low hanging fruit by making some of the fields in struct property and
> >> struct device_node optional. With the changes here, the memory usage goes
> >> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively
> >> small DT.
> >
> > My test case went from 118072 bytes ddown to 21548 bytes with this
> > series.
> 
> Did that include the changes for FDT skipping status=disabled and to
> stop storing the full path of every node? With those 2 alone you said
> it dropped to 25K.

No -- that's with those 6 patches only.

Adding the other 2 patches on top and it is down to 11732 bytes.


Nicolas

Re: [PATCH 0/6] Shrinking DT memory usage

[Nicolas Pitre]

On Fri, 6 Oct 2017, Rob Herring wrote:

> On Fri, Oct 6, 2017 at 4:30 PM, Nicolas Pitre <nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Thu, 5 Oct 2017, Rob Herring wrote:
> >
> >> On kernels with a minimal config and a RAM target in the 100s of KB, DT
> >> is quite a hog of runtime memory usage. How much is dependent on how many
> >> nodes and properties in the DT which have a corresponding struct device_node
> >> and struct property in the kernel. Just skipping disabled nodes saves a
> >> lot by not creating the device_nodes in the first place[1], but there's
> >> more low hanging fruit by making some of the fields in struct property and
> >> struct device_node optional. With the changes here, the memory usage goes
> >> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively
> >> small DT.
> >
> > My test case went from 118072 bytes ddown to 21548 bytes with this
> > series.
> 
> Did that include the changes for FDT skipping status=disabled and to
> stop storing the full path of every node? With those 2 alone you said
> it dropped to 25K.

No -- that's with those 6 patches only.

Adding the other 2 patches on top and it is down to 11732 bytes.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] Shrinking DT memory usage

[Frank Rowand]

On 10/05/17 12:44, Rob Herring wrote:
> On kernels with a minimal config and a RAM target in the 100s of KB, DT 
> is quite a hog of runtime memory usage. How much is dependent on how many
> nodes and properties in the DT which have a corresponding struct device_node
> and struct property in the kernel. Just skipping disabled nodes saves a 
> lot by not creating the device_nodes in the first place[1], but there's 
> more low hanging fruit by making some of the fields in struct property and
> struct device_node optional. With the changes here, the memory usage goes
> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively 
> small DT.
> 
> The majority of the diffstat here is just moving all the kobject/sysfs 
> related code to its own file so we can avoid adding a bunch of ifdefs.
> 
> There's more drastic approaches we could take such as doing the 
> unflattening at build time and storing the bulk of the unflattened tree 
> as const data. Grant also has some ideas on storing properties as ids 
> instead. He's explained it to me, but I still don't understand it.
> 
> Rob
> 
> [1] https://patchwork.kernel.org/patch/9983203/
> 
> Rob Herring (6):
>   of: fix missing kobject init for !SYSFS && OF_DYNAMIC config
>   of: remove struct property.unique_id for FDT
>   of: make struct property _flags field configurable
>   of: move kobj_to_device_node() into dynamic.c
>   of: wrap accesses to device_node kobject
>   of: make kobject and bin_attribute support configurable
> 
>  drivers/base/core.c     |   2 +-
>  drivers/of/Kconfig      |   4 ++
>  drivers/of/Makefile     |   1 +
>  drivers/of/base.c       | 133 ---------------------------------------
>  drivers/of/dynamic.c    |  27 ++------
>  drivers/of/kobj.c       | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/of/of_private.h |  29 +++++++--
>  include/linux/of.h      |  28 +++++----
>  8 files changed, 215 insertions(+), 173 deletions(-)
>  create mode 100644 drivers/of/kobj.c
> 

For the entire series:

Reviewed-by: Frank Rowand <frowand.list@gmail.com>

I don't know if there will be any conflicts between my series
"[PATCH 00/12] of: overlay: clean up device tree overlay code"
and this series.

If you choose to apply this series first, I will make any needed
changes to my series.

-Frank

Re: [PATCH 0/6] Shrinking DT memory usage

[Frank Rowand]

On 10/05/17 12:44, Rob Herring wrote:
> On kernels with a minimal config and a RAM target in the 100s of KB, DT 
> is quite a hog of runtime memory usage. How much is dependent on how many
> nodes and properties in the DT which have a corresponding struct device_node
> and struct property in the kernel. Just skipping disabled nodes saves a 
> lot by not creating the device_nodes in the first place[1], but there's 
> more low hanging fruit by making some of the fields in struct property and
> struct device_node optional. With the changes here, the memory usage goes
> from 17KB to under 8KB on QEMU's ARM virt machine which is a relatively 
> small DT.
> 
> The majority of the diffstat here is just moving all the kobject/sysfs 
> related code to its own file so we can avoid adding a bunch of ifdefs.
> 
> There's more drastic approaches we could take such as doing the 
> unflattening at build time and storing the bulk of the unflattened tree 
> as const data. Grant also has some ideas on storing properties as ids 
> instead. He's explained it to me, but I still don't understand it.
> 
> Rob
> 
> [1] https://patchwork.kernel.org/patch/9983203/
> 
> Rob Herring (6):
>   of: fix missing kobject init for !SYSFS && OF_DYNAMIC config
>   of: remove struct property.unique_id for FDT
>   of: make struct property _flags field configurable
>   of: move kobj_to_device_node() into dynamic.c
>   of: wrap accesses to device_node kobject
>   of: make kobject and bin_attribute support configurable
> 
>  drivers/base/core.c     |   2 +-
>  drivers/of/Kconfig      |   4 ++
>  drivers/of/Makefile     |   1 +
>  drivers/of/base.c       | 133 ---------------------------------------
>  drivers/of/dynamic.c    |  27 ++------
>  drivers/of/kobj.c       | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/of/of_private.h |  29 +++++++--
>  include/linux/of.h      |  28 +++++----
>  8 files changed, 215 insertions(+), 173 deletions(-)
>  create mode 100644 drivers/of/kobj.c
> 

For the entire series:

Reviewed-by: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

I don't know if there will be any conflicts between my series
"[PATCH 00/12] of: overlay: clean up device tree overlay code"
and this series.

If you choose to apply this series first, I will make any needed
changes to my series.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html