Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1348587 > unrolled thread

[PATCH v13 3/6] of, numa: Add NUMA of binding implementation.

Started byDavid Daney <ddaney.cavm@gmail.com>
First post2016-03-03 00:00 +0100
Last post2016-03-04 01:30 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v13 3/6] of, numa: Add NUMA of binding implementation. David Daney <ddaney.cavm@gmail.com> - 2016-03-03 00:00 +0100
    Re: [PATCH v13 3/6] of, numa: Add NUMA of binding implementation. Rob Herring <robh+dt@kernel.org> - 2016-03-03 04:40 +0100
      Re: [PATCH v13 3/6] of, numa: Add NUMA of binding implementation. Ganapatrao Kulkarni <gpkulkarni@gmail.com> - 2016-03-03 05:30 +0100
        Re: [PATCH v13 3/6] of, numa: Add NUMA of binding implementation. Ganapatrao Kulkarni <gpkulkarni@gmail.com> - 2016-03-03 06:20 +0100
      Re: [PATCH v13 3/6] of, numa: Add NUMA of binding implementation. Rob Herring <robh+dt@kernel.org> - 2016-03-04 01:30 +0100

#1348587 — [PATCH v13 3/6] of, numa: Add NUMA of binding implementation.

FromDavid Daney <ddaney.cavm@gmail.com>
Date2016-03-03 00:00 +0100
Subject[PATCH v13 3/6] of, numa: Add NUMA of binding implementation.
Message-ID<r8nUm-6fn-3@gated-at.bofh.it>
From: David Daney <david.daney@cavium.com>

Add device tree parsing for NUMA topology using device
"numa-node-id" property in distance-map and cpu nodes.

This is a complete rewrite of a previous patch by:
   Ganapatrao Kulkarni<gkulkarni@caviumnetworks.com>

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/of/Kconfig   |   3 +
 drivers/of/Makefile  |   1 +
 drivers/of/of_numa.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h   |   9 +++
 4 files changed, 213 insertions(+)
 create mode 100644 drivers/of/of_numa.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index e2a4841..b3bec3a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -112,4 +112,7 @@ config OF_OVERLAY
 	  While this option is selected automatically when needed, you can
 	  enable it manually to improve device tree unit test coverage.
 
+config OF_NUMA
+	bool
+
 endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 156c072..bee3fa9 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD)	+= of_mtd.o
 obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
 obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
+obj-$(CONFIG_OF_NUMA) += of_numa.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
new file mode 100644
index 0000000..9727b60
--- /dev/null
+++ b/drivers/of/of_numa.c
@@ -0,0 +1,200 @@
+/*
+ * OF NUMA Parsing support.
+ *
+ * Copyright (C) 2015 - 2016 Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/nodemask.h>
+
+#include <asm/numa.h>
+
+/* define default numa node to 0 */
+#define DEFAULT_NODE 0
+
+/*
+ * Even though we connect cpus to numa domains later in SMP
+ * init, we need to know the node ids now for all cpus.
+*/
+static void __init of_find_cpu_nodes(void)
+{
+	u32 nid;
+	int r;
+	struct device_node *np = NULL;
+
+	for (;;) {
+		np = of_find_node_by_type(np, "cpu");
+		if (!np)
+			break;
+
+		r = of_property_read_u32(np, "numa-node-id", &nid);
+		if (r)
+			continue;
+
+		pr_debug("NUMA: CPU on %u\n", nid);
+		if (nid >= MAX_NUMNODES)
+			pr_warn("NUMA: Node id %u exceeds maximum value\n",
+				nid);
+		else
+			node_set(nid, numa_nodes_parsed);
+	}
+}
+
+static void __init of_parse_memory_nodes(void)
+{
+	struct device_node *np = NULL;
+	int na, ns;
+	const __be32 *prop;
+	unsigned int psize;
+	u32 nid;
+	u64 base, size;
+	int r;
+
+	for (;;) {
+		np = of_find_node_by_type(np, "memory");
+		if (!np)
+			break;
+
+		r = of_property_read_u32(np, "numa-node-id", &nid);
+		if (r)
+			continue;
+
+		prop = of_get_property(np, "reg", &psize);
+		if (!prop)
+			continue;
+
+		psize /= sizeof(__be32);
+		na = of_n_addr_cells(np);
+		ns = of_n_size_cells(np);
+
+		if (psize < na + ns) {
+			pr_err("NUMA: memory reg property too small\n");
+			continue;
+		}
+		base = of_read_number(prop, na);
+		size = of_read_number(prop + na, ns);
+
+		pr_debug("NUMA:  base = %llx len = %llx, node = %u\n",
+			 base, size, nid);
+
+		if (numa_add_memblk(nid, base, size) < 0)
+			break;
+	}
+
+	of_node_put(np);
+}
+
+static int __init parse_distance_map_v1(struct device_node *map)
+{
+	const __be32 *matrix;
+	unsigned int matrix_size;
+	int entry_count;
+	int i;
+	int nr_size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
+
+	pr_info("NUMA: parsing numa-distance-map-v1\n");
+
+	matrix = of_get_property(map, "distance-matrix", &matrix_size);
+	if (!matrix) {
+		pr_err("NUMA: No distance-matrix property in distance-map\n");
+		return -EINVAL;
+	}
+
+	entry_count = matrix_size / (sizeof(__be32) * 3 * nr_size_cells);
+
+	for (i = 0; i < entry_count; i++) {
+		u32 nodea, nodeb, distance;
+
+		nodea = of_read_number(matrix, nr_size_cells);
+		matrix += nr_size_cells;
+		nodeb = of_read_number(matrix, nr_size_cells);
+		matrix += nr_size_cells;
+		distance = of_read_number(matrix, nr_size_cells);
+		matrix += nr_size_cells;
+
+		numa_set_distance(nodea, nodeb, distance);
+		pr_debug("NUMA:  distance[node%d -> node%d] = %d\n",
+			 nodea, nodeb, distance);
+
+		/* Set default distance of node B->A same as A->B */
+		if (nodeb > nodea)
+			numa_set_distance(nodeb, nodea, distance);
+	}
+
+	return 0;
+}
+
+static int __init of_parse_distance_map(void)
+{
+	int ret = -EINVAL;
+	struct device_node *np = of_find_node_by_name(NULL, "distance-map");
+
+	if (!np)
+		return ret;
+
+	if (of_device_is_compatible(np, "numa-distance-map-v1")) {
+		ret = parse_distance_map_v1(np);
+		goto out;
+	}
+
+	pr_err("NUMA: invalid distance-map device node\n");
+out:
+	of_node_put(np);
+	return ret;
+}
+
+int of_node_to_nid(struct device_node *device)
+{
+	struct device_node *np;
+	u32 nid;
+	int r = -ENODATA;
+
+	np = of_node_get(device);
+
+	while (np) {
+		struct device_node *parent;
+
+		r = of_property_read_u32(np, "numa-node-id", &nid);
+		if (r != -EINVAL)
+			break;
+
+		/* property doesn't exist in this node, look in parent */
+		parent = of_get_parent(np);
+		of_node_put(np);
+		np = parent;
+	}
+	if (np && r)
+		pr_warn("NUMA: Invalid \"numa-node-id\" property in node %s\n",
+			np->name);
+	of_node_put(np);
+
+	if (!r) {
+		if (nid >= MAX_NUMNODES)
+			pr_warn("NUMA: Node id %u exceeds maximum value\n",
+				nid);
+		else
+			return nid;
+	}
+
+	return NUMA_NO_NODE;
+}
+
+int __init of_numa_init(void)
+{
+	of_find_cpu_nodes();
+	of_parse_memory_nodes();
+	return of_parse_distance_map();
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index dc6e396..fe67a4c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -685,6 +685,15 @@ static inline int of_node_to_nid(struct device_node *device)
 }
 #endif
 
+#ifdef CONFIG_OF_NUMA
+extern int of_numa_init(void);
+#else
+static inline int of_numa_init(void)
+{
+	return -ENOSYS;
+}
+#endif
+
 static inline struct device_node *of_find_matching_node(
 	struct device_node *from,
 	const struct of_device_id *matches)
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1348750

FromRob Herring <robh+dt@kernel.org>
Date2016-03-03 04:40 +0100
Message-ID<r8shk-Zf-29@gated-at.bofh.it>
In reply to#1348587
On Wed, Mar 2, 2016 at 4:55 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
>
> Add device tree parsing for NUMA topology using device
> "numa-node-id" property in distance-map and cpu nodes.
>
> This is a complete rewrite of a previous patch by:
>    Ganapatrao Kulkarni<gkulkarni@caviumnetworks.com>
>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/of/Kconfig   |   3 +
>  drivers/of/Makefile  |   1 +
>  drivers/of/of_numa.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h   |   9 +++
>  4 files changed, 213 insertions(+)
>  create mode 100644 drivers/of/of_numa.c
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index e2a4841..b3bec3a 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -112,4 +112,7 @@ config OF_OVERLAY
>           While this option is selected automatically when needed, you can
>           enable it manually to improve device tree unit test coverage.
>
> +config OF_NUMA
> +       bool
> +
>  endif # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 156c072..bee3fa9 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD)  += of_mtd.o
>  obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>  obj-$(CONFIG_OF_RESOLVE)  += resolver.o
>  obj-$(CONFIG_OF_OVERLAY) += overlay.o
> +obj-$(CONFIG_OF_NUMA) += of_numa.o
>
>  obj-$(CONFIG_OF_UNITTEST) += unittest-data/
> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
> new file mode 100644
> index 0000000..9727b60
> --- /dev/null
> +++ b/drivers/of/of_numa.c
> @@ -0,0 +1,200 @@
> +/*
> + * OF NUMA Parsing support.
> + *
> + * Copyright (C) 2015 - 2016 Cavium Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>

This can be dropped now.

> +#include <linux/nodemask.h>
> +
> +#include <asm/numa.h>
> +
> +/* define default numa node to 0 */
> +#define DEFAULT_NODE 0
> +
> +/*
> + * Even though we connect cpus to numa domains later in SMP
> + * init, we need to know the node ids now for all cpus.
> +*/
> +static void __init of_find_cpu_nodes(void)

Perhaps of_parse_cpu_nodes for consistency.

Actually, if all the functions were prefixed with "of_numa_" that
would be better.

> +{
> +       u32 nid;
> +       int r;
> +       struct device_node *np = NULL;
> +
> +       for (;;) {
> +               np = of_find_node_by_type(np, "cpu");
> +               if (!np)
> +                       break;

Can't we use the child node iterator for /cpus here?

> +
> +               r = of_property_read_u32(np, "numa-node-id", &nid);
> +               if (r)
> +                       continue;
> +
> +               pr_debug("NUMA: CPU on %u\n", nid);
> +               if (nid >= MAX_NUMNODES)
> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
> +                               nid);
> +               else
> +                       node_set(nid, numa_nodes_parsed);

I'm not sure how this works, but don't you need to match this up with
MPIDR of the cpu here?

> +       }
> +}
> +
> +static void __init of_parse_memory_nodes(void)
> +{
> +       struct device_node *np = NULL;
> +       int na, ns;
> +       const __be32 *prop;
> +       unsigned int psize;
> +       u32 nid;
> +       u64 base, size;
> +       int r;
> +
> +       for (;;) {
> +               np = of_find_node_by_type(np, "memory");
> +               if (!np)
> +                       break;
> +
> +               r = of_property_read_u32(np, "numa-node-id", &nid);
> +               if (r)
> +                       continue;
> +

> +               prop = of_get_property(np, "reg", &psize);
> +               if (!prop)
> +                       continue;
> +
> +               psize /= sizeof(__be32);
> +               na = of_n_addr_cells(np);
> +               ns = of_n_size_cells(np);
> +
> +               if (psize < na + ns) {
> +                       pr_err("NUMA: memory reg property too small\n");
> +                       continue;
> +               }
> +               base = of_read_number(prop, na);
> +               size = of_read_number(prop + na, ns);

You should be able to use of_address_to_resource for all this.

> +
> +               pr_debug("NUMA:  base = %llx len = %llx, node = %u\n",
> +                        base, size, nid);
> +
> +               if (numa_add_memblk(nid, base, size) < 0)
> +                       break;
> +       }
> +
> +       of_node_put(np);
> +}
> +
> +static int __init parse_distance_map_v1(struct device_node *map)
> +{
> +       const __be32 *matrix;
> +       unsigned int matrix_size;
> +       int entry_count;
> +       int i;
> +       int nr_size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;

I believe the defaults are for some old DT files. As this is new, it
should rely on explicit #size-cells in the DT.

OTOH, what is point of using #size-cells at all versus fixing the
sizes to 1 cell. The documentation doesn't indicate that it uses
#size-cells. That also means that the sizes basically follow the cell
size for the memory given that this is at the top-level.

> +
> +       pr_info("NUMA: parsing numa-distance-map-v1\n");
> +
> +       matrix = of_get_property(map, "distance-matrix", &matrix_size);
> +       if (!matrix) {
> +               pr_err("NUMA: No distance-matrix property in distance-map\n");
> +               return -EINVAL;
> +       }
> +
> +       entry_count = matrix_size / (sizeof(__be32) * 3 * nr_size_cells);
> +
> +       for (i = 0; i < entry_count; i++) {
> +               u32 nodea, nodeb, distance;
> +
> +               nodea = of_read_number(matrix, nr_size_cells);
> +               matrix += nr_size_cells;
> +               nodeb = of_read_number(matrix, nr_size_cells);
> +               matrix += nr_size_cells;
> +               distance = of_read_number(matrix, nr_size_cells);
> +               matrix += nr_size_cells;

Assuming you fix this to 1 cell, you can use
of_property_count_u32_elems and of_property_read_u32_array.

> +
> +               numa_set_distance(nodea, nodeb, distance);
> +               pr_debug("NUMA:  distance[node%d -> node%d] = %d\n",
> +                        nodea, nodeb, distance);
> +
> +               /* Set default distance of node B->A same as A->B */
> +               if (nodeb > nodea)
> +                       numa_set_distance(nodeb, nodea, distance);
> +       }
> +
> +       return 0;
> +}
> +
> +static int __init of_parse_distance_map(void)
> +{
> +       int ret = -EINVAL;
> +       struct device_node *np = of_find_node_by_name(NULL, "distance-map");
> +
> +       if (!np)
> +               return ret;
> +
> +       if (of_device_is_compatible(np, "numa-distance-map-v1")) {

You can use of_find_compatible_node() instead of these 2 calls.

> +               ret = parse_distance_map_v1(np);
> +               goto out;
> +       }
> +
> +       pr_err("NUMA: invalid distance-map device node\n");
> +out:
> +       of_node_put(np);
> +       return ret;
> +}
> +
> +int of_node_to_nid(struct device_node *device)
> +{
> +       struct device_node *np;
> +       u32 nid;
> +       int r = -ENODATA;
> +
> +       np = of_node_get(device);
> +
> +       while (np) {
> +               struct device_node *parent;
> +
> +               r = of_property_read_u32(np, "numa-node-id", &nid);
> +               if (r != -EINVAL)

You want to break for other err values?

> +                       break;
> +
> +               /* property doesn't exist in this node, look in parent */
> +               parent = of_get_parent(np);
> +               of_node_put(np);
> +               np = parent;
> +       }
> +       if (np && r)
> +               pr_warn("NUMA: Invalid \"numa-node-id\" property in node %s\n",
> +                       np->name);
> +       of_node_put(np);
> +
> +       if (!r) {
> +               if (nid >= MAX_NUMNODES)
> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
> +                               nid);
> +               else
> +                       return nid;
> +       }
> +
> +       return NUMA_NO_NODE;
> +}

Needs to be exported?

> +
> +int __init of_numa_init(void)
> +{
> +       of_find_cpu_nodes();
> +       of_parse_memory_nodes();
> +       return of_parse_distance_map();
> +}
> diff --git a/include/linux/of.h b/include/linux/of.h
> index dc6e396..fe67a4c 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -685,6 +685,15 @@ static inline int of_node_to_nid(struct device_node *device)
>  }
>  #endif
>
> +#ifdef CONFIG_OF_NUMA
> +extern int of_numa_init(void);
> +#else
> +static inline int of_numa_init(void)
> +{
> +       return -ENOSYS;
> +}
> +#endif
> +
>  static inline struct device_node *of_find_matching_node(
>         struct device_node *from,
>         const struct of_device_id *matches)
> --
> 1.8.3.1
>

[toc] | [prev] | [next] | [standalone]

#1348773

FromGanapatrao Kulkarni <gpkulkarni@gmail.com>
Date2016-03-03 05:30 +0100
Message-ID<r8t3J-1BU-25@gated-at.bofh.it>
In reply to#1348750
On Thu, Mar 3, 2016 at 9:04 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Mar 2, 2016 at 4:55 PM, David Daney <ddaney.cavm@gmail.com> wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> Add device tree parsing for NUMA topology using device
>> "numa-node-id" property in distance-map and cpu nodes.
>>
>> This is a complete rewrite of a previous patch by:
>>    Ganapatrao Kulkarni<gkulkarni@caviumnetworks.com>
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>  drivers/of/Kconfig   |   3 +
>>  drivers/of/Makefile  |   1 +
>>  drivers/of/of_numa.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of.h   |   9 +++
>>  4 files changed, 213 insertions(+)
>>  create mode 100644 drivers/of/of_numa.c
>>
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index e2a4841..b3bec3a 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -112,4 +112,7 @@ config OF_OVERLAY
>>           While this option is selected automatically when needed, you can
>>           enable it manually to improve device tree unit test coverage.
>>
>> +config OF_NUMA
>> +       bool
>> +
>>  endif # OF
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index 156c072..bee3fa9 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD)  += of_mtd.o
>>  obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>>  obj-$(CONFIG_OF_RESOLVE)  += resolver.o
>>  obj-$(CONFIG_OF_OVERLAY) += overlay.o
>> +obj-$(CONFIG_OF_NUMA) += of_numa.o
>>
>>  obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>> new file mode 100644
>> index 0000000..9727b60
>> --- /dev/null
>> +++ b/drivers/of/of_numa.c
>> @@ -0,0 +1,200 @@
>> +/*
>> + * OF NUMA Parsing support.
>> + *
>> + * Copyright (C) 2015 - 2016 Cavium Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>
> This can be dropped now.
>
>> +#include <linux/nodemask.h>
>> +
>> +#include <asm/numa.h>
>> +
>> +/* define default numa node to 0 */
>> +#define DEFAULT_NODE 0
>> +
>> +/*
>> + * Even though we connect cpus to numa domains later in SMP
>> + * init, we need to know the node ids now for all cpus.
>> +*/
>> +static void __init of_find_cpu_nodes(void)
>
> Perhaps of_parse_cpu_nodes for consistency.
>
> Actually, if all the functions were prefixed with "of_numa_" that
> would be better.
>
>> +{
>> +       u32 nid;
>> +       int r;
>> +       struct device_node *np = NULL;
>> +
>> +       for (;;) {
>> +               np = of_find_node_by_type(np, "cpu");
>> +               if (!np)
>> +                       break;
>
> Can't we use the child node iterator for /cpus here?
>
>> +
>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>> +               if (r)
>> +                       continue;
>> +
>> +               pr_debug("NUMA: CPU on %u\n", nid);
>> +               if (nid >= MAX_NUMNODES)
>> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
>> +                               nid);
>> +               else
>> +                       node_set(nid, numa_nodes_parsed);
>
> I'm not sure how this works, but don't you need to match this up with
> MPIDR of the cpu here?
>
>> +       }
>> +}
>> +
>> +static void __init of_parse_memory_nodes(void)
>> +{
>> +       struct device_node *np = NULL;
>> +       int na, ns;
>> +       const __be32 *prop;
>> +       unsigned int psize;
>> +       u32 nid;
>> +       u64 base, size;
>> +       int r;
>> +
>> +       for (;;) {
>> +               np = of_find_node_by_type(np, "memory");
>> +               if (!np)
>> +                       break;
>> +
>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>> +               if (r)
>> +                       continue;
>> +
>
>> +               prop = of_get_property(np, "reg", &psize);
>> +               if (!prop)
>> +                       continue;
>> +
>> +               psize /= sizeof(__be32);
>> +               na = of_n_addr_cells(np);
>> +               ns = of_n_size_cells(np);
>> +
>> +               if (psize < na + ns) {
>> +                       pr_err("NUMA: memory reg property too small\n");
>> +                       continue;
>> +               }
>> +               base = of_read_number(prop, na);
>> +               size = of_read_number(prop + na, ns);
>
> You should be able to use of_address_to_resource for all this.
>
>> +
>> +               pr_debug("NUMA:  base = %llx len = %llx, node = %u\n",
>> +                        base, size, nid);
>> +
>> +               if (numa_add_memblk(nid, base, size) < 0)
>> +                       break;
>> +       }
>> +
>> +       of_node_put(np);
>> +}
>> +
>> +static int __init parse_distance_map_v1(struct device_node *map)
>> +{
>> +       const __be32 *matrix;
>> +       unsigned int matrix_size;
>> +       int entry_count;
>> +       int i;
>> +       int nr_size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
>
> I believe the defaults are for some old DT files. As this is new, it
> should rely on explicit #size-cells in the DT.
>
> OTOH, what is point of using #size-cells at all versus fixing the
> sizes to 1 cell. The documentation doesn't indicate that it uses
> #size-cells. That also means that the sizes basically follow the cell
> size for the memory given that this is at the top-level.

This property needs only size-cell of 1 (32 bit is sufficient to
define numa node id and their relative distance)
adding note about cell size of this property in binding will clear the doubt.
>
>> +
>> +       pr_info("NUMA: parsing numa-distance-map-v1\n");
>> +
>> +       matrix = of_get_property(map, "distance-matrix", &matrix_size);
>> +       if (!matrix) {
>> +               pr_err("NUMA: No distance-matrix property in distance-map\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       entry_count = matrix_size / (sizeof(__be32) * 3 * nr_size_cells);
>> +
>> +       for (i = 0; i < entry_count; i++) {
>> +               u32 nodea, nodeb, distance;
>> +
>> +               nodea = of_read_number(matrix, nr_size_cells);
>> +               matrix += nr_size_cells;
>> +               nodeb = of_read_number(matrix, nr_size_cells);
>> +               matrix += nr_size_cells;
>> +               distance = of_read_number(matrix, nr_size_cells);
>> +               matrix += nr_size_cells;
>
> Assuming you fix this to 1 cell, you can use
> of_property_count_u32_elems and of_property_read_u32_array.
>
>> +
>> +               numa_set_distance(nodea, nodeb, distance);
>> +               pr_debug("NUMA:  distance[node%d -> node%d] = %d\n",
>> +                        nodea, nodeb, distance);
>> +
>> +               /* Set default distance of node B->A same as A->B */
>> +               if (nodeb > nodea)
>> +                       numa_set_distance(nodeb, nodea, distance);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int __init of_parse_distance_map(void)
>> +{
>> +       int ret = -EINVAL;
>> +       struct device_node *np = of_find_node_by_name(NULL, "distance-map");
>> +
>> +       if (!np)
>> +               return ret;
>> +
>> +       if (of_device_is_compatible(np, "numa-distance-map-v1")) {
>
> You can use of_find_compatible_node() instead of these 2 calls.
>
>> +               ret = parse_distance_map_v1(np);
>> +               goto out;
>> +       }
>> +
>> +       pr_err("NUMA: invalid distance-map device node\n");
>> +out:
>> +       of_node_put(np);
>> +       return ret;
>> +}
>> +
>> +int of_node_to_nid(struct device_node *device)
>> +{
>> +       struct device_node *np;
>> +       u32 nid;
>> +       int r = -ENODATA;
>> +
>> +       np = of_node_get(device);
>> +
>> +       while (np) {
>> +               struct device_node *parent;
>> +
>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>> +               if (r != -EINVAL)
>
> You want to break for other err values?
>
>> +                       break;
>> +
>> +               /* property doesn't exist in this node, look in parent */
>> +               parent = of_get_parent(np);
>> +               of_node_put(np);
>> +               np = parent;
>> +       }
>> +       if (np && r)
>> +               pr_warn("NUMA: Invalid \"numa-node-id\" property in node %s\n",
>> +                       np->name);
>> +       of_node_put(np);
>> +
>> +       if (!r) {
>> +               if (nid >= MAX_NUMNODES)
>> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
>> +                               nid);
>> +               else
>> +                       return nid;
>> +       }
>> +
>> +       return NUMA_NO_NODE;
>> +}
>
> Needs to be exported?
>
>> +
>> +int __init of_numa_init(void)
>> +{
>> +       of_find_cpu_nodes();
>> +       of_parse_memory_nodes();
>> +       return of_parse_distance_map();
>> +}
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index dc6e396..fe67a4c 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -685,6 +685,15 @@ static inline int of_node_to_nid(struct device_node *device)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_OF_NUMA
>> +extern int of_numa_init(void);
>> +#else
>> +static inline int of_numa_init(void)
>> +{
>> +       return -ENOSYS;
>> +}
>> +#endif
>> +
>>  static inline struct device_node *of_find_matching_node(
>>         struct device_node *from,
>>         const struct of_device_id *matches)
>> --
>> 1.8.3.1
>>
>

thanks
Ganapat

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [prev] | [next] | [standalone]

#1348788

FromGanapatrao Kulkarni <gpkulkarni@gmail.com>
Date2016-03-03 06:20 +0100
Message-ID<r8tQ5-29y-1@gated-at.bofh.it>
In reply to#1348773
On Thu, Mar 3, 2016 at 9:55 AM, Ganapatrao Kulkarni
<gpkulkarni@gmail.com> wrote:
> On Thu, Mar 3, 2016 at 9:04 AM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Wed, Mar 2, 2016 at 4:55 PM, David Daney <ddaney.cavm@gmail.com> wrote:
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> Add device tree parsing for NUMA topology using device
>>> "numa-node-id" property in distance-map and cpu nodes.
>>>
>>> This is a complete rewrite of a previous patch by:
>>>    Ganapatrao Kulkarni<gkulkarni@caviumnetworks.com>
>>>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>  drivers/of/Kconfig   |   3 +
>>>  drivers/of/Makefile  |   1 +
>>>  drivers/of/of_numa.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/of.h   |   9 +++
>>>  4 files changed, 213 insertions(+)
>>>  create mode 100644 drivers/of/of_numa.c
>>>
>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>> index e2a4841..b3bec3a 100644
>>> --- a/drivers/of/Kconfig
>>> +++ b/drivers/of/Kconfig
>>> @@ -112,4 +112,7 @@ config OF_OVERLAY
>>>           While this option is selected automatically when needed, you can
>>>           enable it manually to improve device tree unit test coverage.
>>>
>>> +config OF_NUMA
>>> +       bool
>>> +
>>>  endif # OF
>>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>>> index 156c072..bee3fa9 100644
>>> --- a/drivers/of/Makefile
>>> +++ b/drivers/of/Makefile
>>> @@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD)  += of_mtd.o
>>>  obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>>>  obj-$(CONFIG_OF_RESOLVE)  += resolver.o
>>>  obj-$(CONFIG_OF_OVERLAY) += overlay.o
>>> +obj-$(CONFIG_OF_NUMA) += of_numa.o
>>>
>>>  obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>>> new file mode 100644
>>> index 0000000..9727b60
>>> --- /dev/null
>>> +++ b/drivers/of/of_numa.c
>>> @@ -0,0 +1,200 @@
>>> +/*
>>> + * OF NUMA Parsing support.
>>> + *
>>> + * Copyright (C) 2015 - 2016 Cavium Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/of.h>
>>> +#include <linux/of_fdt.h>
>>
>> This can be dropped now.
>>
>>> +#include <linux/nodemask.h>
>>> +
>>> +#include <asm/numa.h>
>>> +
>>> +/* define default numa node to 0 */
>>> +#define DEFAULT_NODE 0
>>> +
>>> +/*
>>> + * Even though we connect cpus to numa domains later in SMP
>>> + * init, we need to know the node ids now for all cpus.
>>> +*/
>>> +static void __init of_find_cpu_nodes(void)
>>
>> Perhaps of_parse_cpu_nodes for consistency.
>>
>> Actually, if all the functions were prefixed with "of_numa_" that
>> would be better.
>>
>>> +{
>>> +       u32 nid;
>>> +       int r;
>>> +       struct device_node *np = NULL;
>>> +
>>> +       for (;;) {
>>> +               np = of_find_node_by_type(np, "cpu");
>>> +               if (!np)
>>> +                       break;
>>
>> Can't we use the child node iterator for /cpus here?
>>
>>> +
>>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>>> +               if (r)
>>> +                       continue;
>>> +
>>> +               pr_debug("NUMA: CPU on %u\n", nid);
>>> +               if (nid >= MAX_NUMNODES)
>>> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
>>> +                               nid);
>>> +               else
>>> +                       node_set(nid, numa_nodes_parsed);
>>
>> I'm not sure how this works, but don't you need to match this up with
>> MPIDR of the cpu here?

we are parsing cpu node to know the number of nodes present in the system.
alone memory will not convey the number of numa nodes present in the
system(in some cases like node has cpu but no memory).
we do mapping of cpu logical id and numa node id in smp init when cpu
logical id is numbered(of_parse_and_init_cpus).
and logical id is already mapped to hwid/MPIDR.

>>
>>> +       }
>>> +}
>>> +
>>> +static void __init of_parse_memory_nodes(void)
>>> +{
>>> +       struct device_node *np = NULL;
>>> +       int na, ns;
>>> +       const __be32 *prop;
>>> +       unsigned int psize;
>>> +       u32 nid;
>>> +       u64 base, size;
>>> +       int r;
>>> +
>>> +       for (;;) {
>>> +               np = of_find_node_by_type(np, "memory");
>>> +               if (!np)
>>> +                       break;
>>> +
>>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>>> +               if (r)
>>> +                       continue;
>>> +
>>
>>> +               prop = of_get_property(np, "reg", &psize);
>>> +               if (!prop)
>>> +                       continue;
>>> +
>>> +               psize /= sizeof(__be32);
>>> +               na = of_n_addr_cells(np);
>>> +               ns = of_n_size_cells(np);
>>> +
>>> +               if (psize < na + ns) {
>>> +                       pr_err("NUMA: memory reg property too small\n");
>>> +                       continue;
>>> +               }
>>> +               base = of_read_number(prop, na);
>>> +               size = of_read_number(prop + na, ns);
>>
>> You should be able to use of_address_to_resource for all this.
>>
>>> +
>>> +               pr_debug("NUMA:  base = %llx len = %llx, node = %u\n",
>>> +                        base, size, nid);
>>> +
>>> +               if (numa_add_memblk(nid, base, size) < 0)
>>> +                       break;
>>> +       }
>>> +
>>> +       of_node_put(np);
>>> +}
>>> +
>>> +static int __init parse_distance_map_v1(struct device_node *map)
>>> +{
>>> +       const __be32 *matrix;
>>> +       unsigned int matrix_size;
>>> +       int entry_count;
>>> +       int i;
>>> +       int nr_size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
>>
>> I believe the defaults are for some old DT files. As this is new, it
>> should rely on explicit #size-cells in the DT.
>>
>> OTOH, what is point of using #size-cells at all versus fixing the
>> sizes to 1 cell. The documentation doesn't indicate that it uses
>> #size-cells. That also means that the sizes basically follow the cell
>> size for the memory given that this is at the top-level.
>
> This property needs only size-cell of 1 (32 bit is sufficient to
> define numa node id and their relative distance)
> adding note about cell size of this property in binding will clear the doubt.
>>
>>> +
>>> +       pr_info("NUMA: parsing numa-distance-map-v1\n");
>>> +
>>> +       matrix = of_get_property(map, "distance-matrix", &matrix_size);
>>> +       if (!matrix) {
>>> +               pr_err("NUMA: No distance-matrix property in distance-map\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       entry_count = matrix_size / (sizeof(__be32) * 3 * nr_size_cells);
>>> +
>>> +       for (i = 0; i < entry_count; i++) {
>>> +               u32 nodea, nodeb, distance;
>>> +
>>> +               nodea = of_read_number(matrix, nr_size_cells);
>>> +               matrix += nr_size_cells;
>>> +               nodeb = of_read_number(matrix, nr_size_cells);
>>> +               matrix += nr_size_cells;
>>> +               distance = of_read_number(matrix, nr_size_cells);
>>> +               matrix += nr_size_cells;
>>
>> Assuming you fix this to 1 cell, you can use
>> of_property_count_u32_elems and of_property_read_u32_array.
>>
>>> +
>>> +               numa_set_distance(nodea, nodeb, distance);
>>> +               pr_debug("NUMA:  distance[node%d -> node%d] = %d\n",
>>> +                        nodea, nodeb, distance);
>>> +
>>> +               /* Set default distance of node B->A same as A->B */
>>> +               if (nodeb > nodea)
>>> +                       numa_set_distance(nodeb, nodea, distance);
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int __init of_parse_distance_map(void)
>>> +{
>>> +       int ret = -EINVAL;
>>> +       struct device_node *np = of_find_node_by_name(NULL, "distance-map");
>>> +
>>> +       if (!np)
>>> +               return ret;
>>> +
>>> +       if (of_device_is_compatible(np, "numa-distance-map-v1")) {
>>
>> You can use of_find_compatible_node() instead of these 2 calls.
>>
>>> +               ret = parse_distance_map_v1(np);
>>> +               goto out;
>>> +       }
>>> +
>>> +       pr_err("NUMA: invalid distance-map device node\n");
>>> +out:
>>> +       of_node_put(np);
>>> +       return ret;
>>> +}
>>> +
>>> +int of_node_to_nid(struct device_node *device)
>>> +{
>>> +       struct device_node *np;
>>> +       u32 nid;
>>> +       int r = -ENODATA;
>>> +
>>> +       np = of_node_get(device);
>>> +
>>> +       while (np) {
>>> +               struct device_node *parent;
>>> +
>>> +               r = of_property_read_u32(np, "numa-node-id", &nid);
>>> +               if (r != -EINVAL)
>>
>> You want to break for other err values?
>>
>>> +                       break;
>>> +
>>> +               /* property doesn't exist in this node, look in parent */
>>> +               parent = of_get_parent(np);
>>> +               of_node_put(np);
>>> +               np = parent;
>>> +       }
>>> +       if (np && r)
>>> +               pr_warn("NUMA: Invalid \"numa-node-id\" property in node %s\n",
>>> +                       np->name);
>>> +       of_node_put(np);
>>> +
>>> +       if (!r) {
>>> +               if (nid >= MAX_NUMNODES)
>>> +                       pr_warn("NUMA: Node id %u exceeds maximum value\n",
>>> +                               nid);
>>> +               else
>>> +                       return nid;
>>> +       }
>>> +
>>> +       return NUMA_NO_NODE;
>>> +}
>>
>> Needs to be exported?
>>
>>> +
>>> +int __init of_numa_init(void)
>>> +{
>>> +       of_find_cpu_nodes();
>>> +       of_parse_memory_nodes();
>>> +       return of_parse_distance_map();
>>> +}
>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>> index dc6e396..fe67a4c 100644
>>> --- a/include/linux/of.h
>>> +++ b/include/linux/of.h
>>> @@ -685,6 +685,15 @@ static inline int of_node_to_nid(struct device_node *device)
>>>  }
>>>  #endif
>>>
>>> +#ifdef CONFIG_OF_NUMA
>>> +extern int of_numa_init(void);
>>> +#else
>>> +static inline int of_numa_init(void)
>>> +{
>>> +       return -ENOSYS;
>>> +}
>>> +#endif
>>> +
>>>  static inline struct device_node *of_find_matching_node(
>>>         struct device_node *from,
>>>         const struct of_device_id *matches)
>>> --
>>> 1.8.3.1
>>>
>>

Ganapat
>
> thanks
> Ganapat
>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [prev] | [next] | [standalone]

#1349726

FromRob Herring <robh+dt@kernel.org>
Date2016-03-04 01:30 +0100
Message-ID<r8LMZ-6Mi-1@gated-at.bofh.it>
In reply to#1348750
On Thu, Mar 3, 2016 at 11:33 AM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 03/02/2016 07:34 PM, Rob Herring wrote:
>>
>> On Wed, Mar 2, 2016 at 4:55 PM, David Daney <ddaney.cavm@gmail.com> wrote:
>>>
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> Add device tree parsing for NUMA topology using device
>>> "numa-node-id" property in distance-map and cpu nodes.
>>>
>>> This is a complete rewrite of a previous patch by:
>>>     Ganapatrao Kulkarni<gkulkarni@caviumnetworks.com>
>>>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>   drivers/of/Kconfig   |   3 +
>>>   drivers/of/Makefile  |   1 +
>>>   drivers/of/of_numa.c | 200
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/of.h   |   9 +++
>>>   4 files changed, 213 insertions(+)
>>>   create mode 100644 drivers/of/of_numa.c

[...]

>>> +static int __init of_parse_distance_map(void)
>>> +{
>>> +       int ret = -EINVAL;
>>> +       struct device_node *np = of_find_node_by_name(NULL,
>>> "distance-map");
>>> +
>>> +       if (!np)
>>> +               return ret;
>>> +
>>> +       if (of_device_is_compatible(np, "numa-distance-map-v1")) {
>>
>>
>> You can use of_find_compatible_node() instead of these 2 calls.
>
>
> Well, we need to match exactly the name "distance-map",
> of_find_compatible_node() doesn't match on the name, so I think we need two
> checks, one for name and one for compatible.

Node names generally aren't supposed to matter and the kernel's job is
not to validate DTs, So just finding the compatible should be enough.

Not a big deal, so I've acked v14. If there's a v15, then please make
the change.

Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web