[RFC PATCH v3 1/4] arm64: defconfig: increase NR_CPUS range to 2-4096.

[RFC PATCH v3 1/4] arm64: defconfig: increase NR_CPUS range to 2-4096.

[Ganapatrao Kulkarni]

Raising the maximum limit to 4096.
This is to accomadate up-coming higher multi-core platforms.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>
---
 arch/arm64/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 61fbd1f..242419d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -302,8 +302,8 @@ config SCHED_SMT
 	  places. If unsure say N here.
 
 config NR_CPUS
-	int "Maximum number of CPUs (2-64)"
-	range 2 64
+	int "Maximum number of CPUs (2-4096)"
+	range 2 4096
 	depends on SMP
 	# These have to remain sorted largest to smallest
 	default "64"
-- 
1.8.1.4

[RFC PATCH v3 4/4] arm64:numa: adding numa support for arm64 platforms.

[Arnd Bergmann]

[re-sent with correct mailing list address]

On Wednesday 31 December 2014 13:03:28 Ganapatrao Kulkarni wrote:
> Adding numa support for arm64 based platforms.
> Adding dt node pasring for numa topology using property arm,associativity.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>

Maybe the parts that are common with powerpc can be moved to drivers/of/numa.c?
We can always look for both arm,associativity and ibm,associativity, I don't
think we should be worried about any conflicts that way.

> +#define MAX_DISTANCE_REF_POINTS 4

I think we should use 8 here like powerpc, four levels might get exceeded
on complex SoCs.

> +int dt_get_cpu_node_id(int cpu)
> +{
> +	struct device_node *dn = NULL;
> +
> +	while ((dn = of_find_node_by_type(dn, "cpu"))) {
> +		const u32 *cell;
> +		u64 hwid;
> +
> +		/*
> +		 * A cpu node with missing "reg" property is
> +		 * considered invalid to build a cpu_logical_map
> +		 * entry.
> +		 */
> +		cell = of_get_property(dn, "reg", NULL);
> +		if (!cell) {
> +			pr_err("%s: missing reg property\n", dn->full_name);
> +			return default_nid;
> +		}
> +		hwid = of_read_number(cell, of_n_addr_cells(dn));
> +
> +		if (cpu_logical_map(cpu) == hwid)
> +		return of_node_to_nid_single(dn);
> +	}
> +	return NUMA_NO_NODE;
> +}
> +EXPORT_SYMBOL(dt_get_cpu_node_id);

Maybe just expose a function to the device node for a CPU ID here, and
expect callers to use of_node_to_nid?

> +
> +/**
> + * early_init_dt_scan_numa_map - parse memory node and map nid to memory range.
> + */
> +int __init early_init_dt_scan_numa_map(unsigned long node, const char *uname,
> +				     int depth, void *data)
> +{
> +	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> +
> +	/* We are scanning "numa-map" nodes only */

a stale comment?

> +/* DT node mapping is done already early_init_dt_scan_memory */
> +int __init arm64_dt_numa_init(void)
> +{
> +	int i;
> +	u32 nodea, nodeb, distance, node_count = 0;
> +
> +	of_scan_flat_dt(early_init_dt_scan_numa_map, NULL);
> +
> +	for_each_node_mask(i, numa_nodes_parsed)
> +		node_count = i;
> +	node_count++;
> +
> +	for (nodea =  0; nodea < node_count; nodea++) {
> +		for (nodeb = 0; nodeb < node_count; nodeb++) {
> +			distance = dt_get_node_distance(nodea, nodeb);
> +			numa_set_distance(nodea, nodeb, distance);
> +		}
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(arm64_dt_numa_init);

No need to export functions that are called only be architecture code.
Since this works on the flattened device tree format, you can never
have loadable modules calling it.

> @@ -461,7 +464,12 @@ static int c_show(struct seq_file *m, void *v)
>  		 * "processor".  Give glibc what it expects.
>  		 */
>  #ifdef CONFIG_SMP
> +	if (IS_ENABLED(CONFIG_NUMA)) {
> +		seq_printf(m, "processor\t: %d", i);
> +		seq_printf(m, " [nid: %d]\n", cpu_to_node(i));
> +	} else {
>  		seq_printf(m, "processor\t: %d\n", i);
> +	}
>  #endif
>  	}

Do we need to make this conditional? I think we can just always
print the node number, even if it's going to be zero for systems
without the associativity properties.

> +
> +int cpu_to_node_map[NR_CPUS];
> +EXPORT_SYMBOL(cpu_to_node_map);

This seems to be x86 specific, do we need it?

> +/*
> + *  Set the cpu to node and mem mapping
> + */
> +void numa_store_cpu_info(int cpu)
> +{
> +#ifdef CONFIG_ARM64_DT_NUMA
> +	node_cpu_hwid[cpu].node_id  =  dt_get_cpu_node_id(cpu);
> +#endif

I would try to avoid the #ifdef here, by providing a stub function of
dt_get_cpu_node_id or whichever function we end up calling here when
NUMA is disabled.

> +
> +/**
> + * arm64_numa_init - Initialize NUMA
> + *
> + * Try each configured NUMA initialization method until one succeeds.  The
> + * last fallback is dummy single node config encomapssing whole memory and
> + * never fails.
> + */
> +void __init arm64_numa_init(void)
> +{
> +	if (!numa_off) {
> +#ifdef CONFIG_ARM64_DT_NUMA
> +		if (!numa_init(arm64_dt_numa_init))
> +			return;
> +#endif
> +	}
> +
> +	numa_init(dummy_numa_init);
> +}

I don't think we need the CONFIG_ARM64_DT_NUMA=n case here, it should just
not be conditional, and the arm64_dt_numa_init should fall back to doing
something reasonable when numa is turned off or there are no associativity
properties.

	Arnd

[RFC PATCH v3 2/4] Documentation: arm64/arm: dt bindings for numa.

[Arnd Bergmann]

On Wednesday 31 December 2014 13:03:26 Ganapatrao Kulkarni wrote:
> DT bindings for numa map for memory, cores and IOs using arm,associativity
> device node property.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>
> ---
>  Documentation/devicetree/bindings/arm/numa.txt | 198 +++++++++++++++++++++++++
>  1 file changed, 198 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/numa.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/numa.txt b/Documentation/devicetree/bindings/arm/numa.txt
> new file mode 100644
> index 0000000..4f51e25
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/numa.txt
> @@ -0,0 +1,198 @@
> +==============================================================================
> +NUMA binding description.
> +==============================================================================
> +
> +==============================================================================
> +1 - Introduction
> +==============================================================================
> +
> +Systems employing a Non Uniform Memory Access (NUMA) architecture contain
> +collections of hardware resources including processors, memory, and I/O buses,
> +that comprise what is commonly known as a ???NUMA node???.
> +Processor accesses to memory within the local NUMA node is generally faster
> +than processor accesses to memory outside of the local NUMA node.
> +DT defines interfaces that allow the platform to convey NUMA node
> +topology information to OS.
> +
> +==============================================================================
> +2 - arm,associativity
> +==============================================================================
> +
> +The mapping is done using arm,associativity device property.
> +this property needs to be present in every device node which needs to to be
> +mapped to numa nodes.
> +
> +arm,associativity property is set of 32-bit integers. representing the
> +board id, socket id and core id.
> +
> +ex:
> +	/* board 0, socket 0, core 0 */
> +	arm,associativity = <0 0 0x000>;
> +
> +	/* board 1, socket 0, core 8 */
> +	arm,associativity = <1 0 0x08>;

This is way too specific to Cavium machines. Most other vendors will not (at first)
have multiple boards or multiple sockets, but need to represent multiple clusters
and/or SMT threads instead. Also the wording suggests that this is only relevant
for NUMA, which I don't think is helpful because we will also want to describe
the topology within one NUMA node for locality.

I think we should stick to the powerpc definition here and not define what the
levels mean at the binding level. Something like:

"Each level of topology defines a boundary in the system at which a significant
difference in performance can be measured between cross-device accesses within
a single location and those spanning multiple locations. The first cell always
contains the broadest subdivision within the system, while the last cell enumerates
the individual devices, such as an SMT thread of a CPU, or a bus bridge within
an SoC".

> +==============================================================================
> +3 - arm,associativity-reference-points
> +==============================================================================
> +This property is a set of 32-bit integers, each representing an index into
> +the arm,associativity nodes. The first integer is the most significant
> +NUMA boundary and the following are progressively less significant boundaries.
> +There can be more than one level of NUMA.
> +
> +Ex:
> +	arm,associativity-reference-points = <0 1>;
> +	The board Id(index 0) used first to calculate the associativity (node
> +	distance), then follows the  socket id(index 1).
> +
> +	arm,associativity-reference-points = <1 0>;
> +	The socket Id(index 1) used first to calculate the associativity,
> +	then follows the board id(index 0).
> +
> +	arm,associativity-reference-points = <0>;
> +	Only the board Id(index 0) used to calculate the associativity.
> +
> +	arm,associativity-reference-points = <1>;
> +	Only socket Id(index 1) used to calculate the associativity.
> +
> +==============================================================================
> +4 - Example dts
> +==============================================================================
> +
> +Example: 2 Node system consists of 2 boards and each board having one socket
> +and 8 core in each socket.

I think the example should also include a PCI controller.

> +
> +	arm,associativity-reference-points = <0 1>;

This doesn't really match the associativity properties, because the
second level in the cpus nodes is completely meaningless and should
not be listed as a secondary reference point.

	Arnd

[RFC PATCH v3 1/4] arm64: defconfig: increase NR_CPUS range to 2-4096.

[Arnd Bergmann]

On Wednesday 31 December 2014 13:03:25 Ganapatrao Kulkarni wrote:
> Raising the maximum limit to 4096.
> This is to accomadate up-coming higher multi-core platforms.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Arnd Bergmann]

On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
> +
> +	memory at 00000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x00000000 0x0 0x80000000>;
> +		/* board 0, socket 0, no specific core */
> +		arm,associativity = <0 0 0xffff>;
> +	};
> +
> +	memory at 10000000000 {
> +		device_type = "memory";
> +		reg = <0x100 0x00000000 0x0 0x80000000>;
> +		/* board 1, socket 0, no specific core */
> +		arm,associativity = <1 0 0xffff>;
> +	};
> +};

So no memory in any other socket?

> +		cpu at 00f {
> +			device_type = "cpu";
> +			compatible = "cavium,thunder", "arm,armv8";
> +			reg = <0x0 0x00f>;
> +			enable-method = "psci";
> +			arm,associativity = <0 0 0x00f>;
> +		};
> +		cpu at 100 {
> +			device_type = "cpu";
> +			compatible = "cavium,thunder", "arm,armv8";
> +			reg = <0x0 0x100>;
> +			enable-method = "psci";
> +			arm,associativity = <0 0 0x100>;
> +		};

What is the 0x100 offset in the last-level topology field? Does this have
no significance to topology at all? I would expect that to be something
like cluster number that is relevant to caching and should be represented
as a separate level.

In contrast, the level-two topology information seems to always be
zero for all CPUs, so you could probably leave that one out.

> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;

The soc node is missing a topology information, please add one.

	Arnd

[RFC PATCH v3 2/4] Documentation: arm64/arm: dt bindings for numa.

[Ganapatrao Kulkarni]

Hi Arnd,


On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 31 December 2014 13:03:26 Ganapatrao Kulkarni wrote:
>> DT bindings for numa map for memory, cores and IOs using arm,associativity
>> device node property.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>
>> ---
>>  Documentation/devicetree/bindings/arm/numa.txt | 198 +++++++++++++++++++++++++
>>  1 file changed, 198 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/numa.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/numa.txt b/Documentation/devicetree/bindings/arm/numa.txt
>> new file mode 100644
>> index 0000000..4f51e25
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/numa.txt
>> @@ -0,0 +1,198 @@
>> +==============================================================================
>> +NUMA binding description.
>> +==============================================================================
>> +
>> +==============================================================================
>> +1 - Introduction
>> +==============================================================================
>> +
>> +Systems employing a Non Uniform Memory Access (NUMA) architecture contain
>> +collections of hardware resources including processors, memory, and I/O buses,
>> +that comprise what is commonly known as a ???NUMA node?? .
>> +Processor accesses to memory within the local NUMA node is generally faster
>> +than processor accesses to memory outside of the local NUMA node.
>> +DT defines interfaces that allow the platform to convey NUMA node
>> +topology information to OS.
>> +
>> +==============================================================================
>> +2 - arm,associativity
>> +==============================================================================
>> +
>> +The mapping is done using arm,associativity device property.
>> +this property needs to be present in every device node which needs to to be
>> +mapped to numa nodes.
>> +
>> +arm,associativity property is set of 32-bit integers. representing the
>> +board id, socket id and core id.
>> +
>> +ex:
>> +     /* board 0, socket 0, core 0 */
>> +     arm,associativity = <0 0 0x000>;
>> +
>> +     /* board 1, socket 0, core 8 */
>> +     arm,associativity = <1 0 0x08>;
>
> This is way too specific to Cavium machines. Most other vendors will not (at first)
> have multiple boards or multiple sockets, but need to represent multiple clusters
> and/or SMT threads instead. Also the wording suggests that this is only relevant
> for NUMA, which I don't think is helpful because we will also want to describe
> the topology within one NUMA node for locality.
>
> I think we should stick to the powerpc definition here and not define what the
> levels mean at the binding level. Something like:
>
> "Each level of topology defines a boundary in the system at which a significant
> difference in performance can be measured between cross-device accesses within
> a single location and those spanning multiple locations. The first cell always
> contains the broadest subdivision within the system, while the last cell enumerates
> the individual devices, such as an SMT thread of a CPU, or a bus bridge within
> an SoC".
Ok,, i will change as suggested.
>
>> +==============================================================================
>> +3 - arm,associativity-reference-points
>> +==============================================================================
>> +This property is a set of 32-bit integers, each representing an index into
>> +the arm,associativity nodes. The first integer is the most significant
>> +NUMA boundary and the following are progressively less significant boundaries.
>> +There can be more than one level of NUMA.
>> +
>> +Ex:
>> +     arm,associativity-reference-points = <0 1>;
>> +     The board Id(index 0) used first to calculate the associativity (node
>> +     distance), then follows the  socket id(index 1).
>> +
>> +     arm,associativity-reference-points = <1 0>;
>> +     The socket Id(index 1) used first to calculate the associativity,
>> +     then follows the board id(index 0).
>> +
>> +     arm,associativity-reference-points = <0>;
>> +     Only the board Id(index 0) used to calculate the associativity.
>> +
>> +     arm,associativity-reference-points = <1>;
>> +     Only socket Id(index 1) used to calculate the associativity.
>> +
>> +==============================================================================
>> +4 - Example dts
>> +==============================================================================
>> +
>> +Example: 2 Node system consists of 2 boards and each board having one socket
>> +and 8 core in each socket.
>
> I think the example should also include a PCI controller.
Yes, i will add pci.
>
>> +
>> +     arm,associativity-reference-points = <0 1>;
>
> This doesn't really match the associativity properties, because the
> second level in the cpus nodes is completely meaningless and should
> not be listed as a secondary reference point.
agreed, will remove second entry.
>
>         Arnd

thanks
ganapat

[RFC PATCH v3 4/4] arm64:numa: adding numa support for arm64 platforms.

[Ganapatrao Kulkarni]

Hi Arnd,


On Sat, Jan 3, 2015 at 2:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> [re-sent with correct mailing list address]
>
> On Wednesday 31 December 2014 13:03:28 Ganapatrao Kulkarni wrote:
>> Adding numa support for arm64 based platforms.
>> Adding dt node pasring for numa topology using property arm,associativity.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com>
>
> Maybe the parts that are common with powerpc can be moved to drivers/of/numa.c?
> We can always look for both arm,associativity and ibm,associativity, I don't
> think we should be worried about any conflicts that way.
ok, i will move common functions from powerpc and arm64 to driver/of/numa.c
>
>> +#define MAX_DISTANCE_REF_POINTS 4
>
> I think we should use 8 here like powerpc, four levels might get exceeded
> on complex SoCs.
sure.
>
>> +int dt_get_cpu_node_id(int cpu)
>> +{
>> +     struct device_node *dn = NULL;
>> +
>> +     while ((dn = of_find_node_by_type(dn, "cpu"))) {
>> +             const u32 *cell;
>> +             u64 hwid;
>> +
>> +             /*
>> +              * A cpu node with missing "reg" property is
>> +              * considered invalid to build a cpu_logical_map
>> +              * entry.
>> +              */
>> +             cell = of_get_property(dn, "reg", NULL);
>> +             if (!cell) {
>> +                     pr_err("%s: missing reg property\n", dn->full_name);
>> +                     return default_nid;
>> +             }
>> +             hwid = of_read_number(cell, of_n_addr_cells(dn));
>> +
>> +             if (cpu_logical_map(cpu) == hwid)
>> +             return of_node_to_nid_single(dn);
>> +     }
>> +     return NUMA_NO_NODE;
>> +}
>> +EXPORT_SYMBOL(dt_get_cpu_node_id);
>
> Maybe just expose a function to the device node for a CPU ID here, and
> expect callers to use of_node_to_nid?
shall i make this wrapper function in dt_numa.c, which will use
functions _of_node_to_nid and  _of_cpu_to_node(cpu)
And,  this function can be a weak function in numa.c which returns 0.
>
>> +
>> +/**
>> + * early_init_dt_scan_numa_map - parse memory node and map nid to memory range.
>> + */
>> +int __init early_init_dt_scan_numa_map(unsigned long node, const char *uname,
>> +                                  int depth, void *data)
>> +{
>> +     const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>> +
>> +     /* We are scanning "numa-map" nodes only */
>
> a stale comment?
oops, will remove.
>
>> +/* DT node mapping is done already early_init_dt_scan_memory */
>> +int __init arm64_dt_numa_init(void)
>> +{
>> +     int i;
>> +     u32 nodea, nodeb, distance, node_count = 0;
>> +
>> +     of_scan_flat_dt(early_init_dt_scan_numa_map, NULL);
>> +
>> +     for_each_node_mask(i, numa_nodes_parsed)
>> +             node_count = i;
>> +     node_count++;
>> +
>> +     for (nodea =  0; nodea < node_count; nodea++) {
>> +             for (nodeb = 0; nodeb < node_count; nodeb++) {
>> +                     distance = dt_get_node_distance(nodea, nodeb);
>> +                     numa_set_distance(nodea, nodeb, distance);
>> +             }
>> +     }
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(arm64_dt_numa_init);
>
> No need to export functions that are called only be architecture code.
> Since this works on the flattened device tree format, you can never
> have loadable modules calling it.
yes, will do.
>
>> @@ -461,7 +464,12 @@ static int c_show(struct seq_file *m, void *v)
>>                * "processor".  Give glibc what it expects.
>>                */
>>  #ifdef CONFIG_SMP
>> +     if (IS_ENABLED(CONFIG_NUMA)) {
>> +             seq_printf(m, "processor\t: %d", i);
>> +             seq_printf(m, " [nid: %d]\n", cpu_to_node(i));
>> +     } else {
>>               seq_printf(m, "processor\t: %d\n", i);
>> +     }
>>  #endif
>>       }
>
> Do we need to make this conditional? I think we can just always
> print the node number, even if it's going to be zero for systems
> without the associativity properties.
yes, we can.
>
>> +
>> +int cpu_to_node_map[NR_CPUS];
>> +EXPORT_SYMBOL(cpu_to_node_map);
>
> This seems to be x86 specific, do we need it?
>
>> +/*
>> + *  Set the cpu to node and mem mapping
>> + */
>> +void numa_store_cpu_info(int cpu)
>> +{
>> +#ifdef CONFIG_ARM64_DT_NUMA
>> +     node_cpu_hwid[cpu].node_id  =  dt_get_cpu_node_id(cpu);
>> +#endif
>
> I would try to avoid the #ifdef here, by providing a stub function of
> dt_get_cpu_node_id or whichever function we end up calling here when
> NUMA is disabled.
as commented above.
.>
>> +
>> +/**
>> + * arm64_numa_init - Initialize NUMA
>> + *
>> + * Try each configured NUMA initialization method until one succeeds.  The
>> + * last fallback is dummy single node config encomapssing whole memory and
>> + * never fails.
>> + */
>> +void __init arm64_numa_init(void)
>> +{
>> +     if (!numa_off) {
>> +#ifdef CONFIG_ARM64_DT_NUMA
>> +             if (!numa_init(arm64_dt_numa_init))
>> +                     return;
>> +#endif
>> +     }
>> +
>> +     numa_init(dummy_numa_init);
>> +}
>
> I don't think we need the CONFIG_ARM64_DT_NUMA=n case here, it should just
> not be conditional, and the arm64_dt_numa_init should fall back to doing
> something reasonable when numa is turned off or there are no associativity
> properties.
i think we can remove ifdef, will do it.
>
>         Arnd

thanks
ganapat

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Ganapatrao Kulkarni]

On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
>> +
>> +     memory at 00000000 {
>> +             device_type = "memory";
>> +             reg = <0x0 0x00000000 0x0 0x80000000>;
>> +             /* board 0, socket 0, no specific core */
>> +             arm,associativity = <0 0 0xffff>;
>> +     };
>> +
>> +     memory at 10000000000 {
>> +             device_type = "memory";
>> +             reg = <0x100 0x00000000 0x0 0x80000000>;
>> +             /* board 1, socket 0, no specific core */
>> +             arm,associativity = <1 0 0xffff>;
>> +     };
>> +};
>
> So no memory in any other socket?
>
>> +             cpu at 00f {
>> +                     device_type = "cpu";
>> +                     compatible = "cavium,thunder", "arm,armv8";
>> +                     reg = <0x0 0x00f>;
>> +                     enable-method = "psci";
>> +                     arm,associativity = <0 0 0x00f>;
>> +             };
>> +             cpu at 100 {
>> +                     device_type = "cpu";
>> +                     compatible = "cavium,thunder", "arm,armv8";
>> +                     reg = <0x0 0x100>;
>> +                     enable-method = "psci";
>> +                     arm,associativity = <0 0 0x100>;
>> +             };
>
> What is the 0x100 offset in the last-level topology field? Does this have
> no significance to topology at all? I would expect that to be something
> like cluster number that is relevant to caching and should be represented
> as a separate level.
i did not understand, can you please explain little more about "
should be represented as a separate level."
at present, i have put the hwid of a cpu.
>
> In contrast, the level-two topology information seems to always be
> zero for all CPUs, so you could probably leave that one out.
>
>> +     soc {
>> +             compatible = "simple-bus";
>> +             #address-cells = <2>;
>> +             #size-cells = <2>;
>> +             ranges;
>
> The soc node is missing a topology information, please add one.
ok, will be added.
>
>         Arnd

thanks
ganapat

[RFC PATCH v3 4/4] arm64:numa: adding numa support for arm64 platforms.

[Arnd Bergmann]

On Tuesday 06 January 2015 14:55:53 Ganapatrao Kulkarni wrote:
> On Sat, Jan 3, 2015 at 2:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> +int dt_get_cpu_node_id(int cpu)
> >> +{
> >> +     struct device_node *dn = NULL;
> >> +
> >> +     while ((dn = of_find_node_by_type(dn, "cpu"))) {
> >> +             const u32 *cell;
> >> +             u64 hwid;
> >> +
> >> +             /*
> >> +              * A cpu node with missing "reg" property is
> >> +              * considered invalid to build a cpu_logical_map
> >> +              * entry.
> >> +              */
> >> +             cell = of_get_property(dn, "reg", NULL);
> >> +             if (!cell) {
> >> +                     pr_err("%s: missing reg property\n", dn->full_name);
> >> +                     return default_nid;
> >> +             }
> >> +             hwid = of_read_number(cell, of_n_addr_cells(dn));
> >> +
> >> +             if (cpu_logical_map(cpu) == hwid)
> >> +             return of_node_to_nid_single(dn);
> >> +     }
> >> +     return NUMA_NO_NODE;
> >> +}
> >> +EXPORT_SYMBOL(dt_get_cpu_node_id);
> >
> > Maybe just expose a function to the device node for a CPU ID here, and
> > expect callers to use of_node_to_nid?
> shall i make this wrapper function in dt_numa.c, which will use
> functions _of_node_to_nid and  _of_cpu_to_node(cpu)

Yes, I guess that would work.

> And,  this function can be a weak function in numa.c which returns 0.

No, please don't use weak functions. You can either use IS_ENABLED()
tricks to remove function calls at compile-time, or in the header
file provide an inline function as an alternative to the extern
declaration, based on a configuration symbol.

	Arnd

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Arnd Bergmann]

On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
> >> +             cpu at 00f {
> >> +                     device_type = "cpu";
> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> +                     reg = <0x0 0x00f>;
> >> +                     enable-method = "psci";
> >> +                     arm,associativity = <0 0 0x00f>;
> >> +             };
> >> +             cpu at 100 {
> >> +                     device_type = "cpu";
> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> +                     reg = <0x0 0x100>;
> >> +                     enable-method = "psci";
> >> +                     arm,associativity = <0 0 0x100>;
> >> +             };
> >
> > What is the 0x100 offset in the last-level topology field? Does this have
> > no significance to topology at all? I would expect that to be something
> > like cluster number that is relevant to caching and should be represented
> > as a separate level.
>
> i did not understand, can you please explain little more about "
> should be represented as a separate level."
> at present, i have put the hwid of a cpu.

>From what I undertand, the hwid of the CPU contains the "cluster" number in
this bit position, so you typically have a shared L2 or L3 cache between
all cores within a cluster, but separate caches in other clusters.

If this is the case, there will be a measurable difference in performance
between two processes sharing memory when running on the same cluster,
or when running on different clusters on the same socket. If the
performance difference is relevant, it should be described as a separate
level in the associativity property.

	Arnd

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Ganapatrao Kulkarni]

Hi Arnd,

On Wed, Jan 7, 2015 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
>> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
>> >> +             cpu at 00f {
>> >> +                     device_type = "cpu";
>> >> +                     compatible = "cavium,thunder", "arm,armv8";
>> >> +                     reg = <0x0 0x00f>;
>> >> +                     enable-method = "psci";
>> >> +                     arm,associativity = <0 0 0x00f>;
>> >> +             };
>> >> +             cpu at 100 {
>> >> +                     device_type = "cpu";
>> >> +                     compatible = "cavium,thunder", "arm,armv8";
>> >> +                     reg = <0x0 0x100>;
>> >> +                     enable-method = "psci";
>> >> +                     arm,associativity = <0 0 0x100>;
>> >> +             };
>> >
>> > What is the 0x100 offset in the last-level topology field? Does this have
>> > no significance to topology at all? I would expect that to be something
>> > like cluster number that is relevant to caching and should be represented
>> > as a separate level.
>>
>> i did not understand, can you please explain little more about "
>> should be represented as a separate level."
>> at present, i have put the hwid of a cpu.
>
> From what I undertand, the hwid of the CPU contains the "cluster" number in
> this bit position, so you typically have a shared L2 or L3 cache between
> all cores within a cluster, but separate caches in other clusters.
>
> If this is the case, there will be a measurable difference in performance
> between two processes sharing memory when running on the same cluster,
> or when running on different clusters on the same socket. If the
> performance difference is relevant, it should be described as a separate
> level in the associativity property.
you mean, the associativity as array of  <board> <socket> <cluster>
>
>         Arnd
thanks
ganapat

[RFC PATCH v3 4/4] arm64:numa: adding numa support for arm64 platforms.

[Ganapatrao Kulkarni]

On Wed, Jan 7, 2015 at 1:29 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 06 January 2015 14:55:53 Ganapatrao Kulkarni wrote:
>> On Sat, Jan 3, 2015 at 2:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> +int dt_get_cpu_node_id(int cpu)
>> >> +{
>> >> +     struct device_node *dn = NULL;
>> >> +
>> >> +     while ((dn = of_find_node_by_type(dn, "cpu"))) {
>> >> +             const u32 *cell;
>> >> +             u64 hwid;
>> >> +
>> >> +             /*
>> >> +              * A cpu node with missing "reg" property is
>> >> +              * considered invalid to build a cpu_logical_map
>> >> +              * entry.
>> >> +              */
>> >> +             cell = of_get_property(dn, "reg", NULL);
>> >> +             if (!cell) {
>> >> +                     pr_err("%s: missing reg property\n", dn->full_name);
>> >> +                     return default_nid;
>> >> +             }
>> >> +             hwid = of_read_number(cell, of_n_addr_cells(dn));
>> >> +
>> >> +             if (cpu_logical_map(cpu) == hwid)
>> >> +             return of_node_to_nid_single(dn);
>> >> +     }
>> >> +     return NUMA_NO_NODE;
>> >> +}
>> >> +EXPORT_SYMBOL(dt_get_cpu_node_id);
>> >
>> > Maybe just expose a function to the device node for a CPU ID here, and
>> > expect callers to use of_node_to_nid?
>> shall i make this wrapper function in dt_numa.c, which will use
>> functions _of_node_to_nid and  _of_cpu_to_node(cpu)
>
> Yes, I guess that would work.
>
>> And,  this function can be a weak function in numa.c which returns 0.
>
> No, please don't use weak functions. You can either use IS_ENABLED()
> tricks to remove function calls at compile-time, or in the header
> file provide an inline function as an alternative to the extern
> declaration, based on a configuration symbol.
ok
>
>         Arnd
thanks
ganaapat

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Arnd Bergmann]

On Wednesday 07 January 2015 12:37:51 Ganapatrao Kulkarni wrote:
> Hi Arnd,
> 
> On Wed, Jan 7, 2015 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
> >> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
> >> >> +             cpu at 00f {
> >> >> +                     device_type = "cpu";
> >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> >> +                     reg = <0x0 0x00f>;
> >> >> +                     enable-method = "psci";
> >> >> +                     arm,associativity = <0 0 0x00f>;
> >> >> +             };
> >> >> +             cpu at 100 {
> >> >> +                     device_type = "cpu";
> >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> >> +                     reg = <0x0 0x100>;
> >> >> +                     enable-method = "psci";
> >> >> +                     arm,associativity = <0 0 0x100>;
> >> >> +             };
> >> >
> >> > What is the 0x100 offset in the last-level topology field? Does this have
> >> > no significance to topology at all? I would expect that to be something
> >> > like cluster number that is relevant to caching and should be represented
> >> > as a separate level.
> >>
> >> i did not understand, can you please explain little more about "
> >> should be represented as a separate level."
> >> at present, i have put the hwid of a cpu.
> >
> > From what I undertand, the hwid of the CPU contains the "cluster" number in
> > this bit position, so you typically have a shared L2 or L3 cache between
> > all cores within a cluster, but separate caches in other clusters.
> >
> > If this is the case, there will be a measurable difference in performance
> > between two processes sharing memory when running on the same cluster,
> > or when running on different clusters on the same socket. If the
> > performance difference is relevant, it should be described as a separate
> > level in the associativity property.
> you mean, the associativity as array of  <board> <socket> <cluster>

No, that would leave out the core number, which is required to identify
the individual thread. I meant adding an extra level such as

<board> <socket> <cluster> <core>

A lot of machines will leave out the <board> number because they are
built with SoCs that don't have a long-distance coherency protocol.

	Arnd

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Lorenzo Pieralisi]

On Wed, Jan 07, 2015 at 08:18:50AM +0000, Arnd Bergmann wrote:
> On Wednesday 07 January 2015 12:37:51 Ganapatrao Kulkarni wrote:
> > Hi Arnd,
> > 
> > On Wed, Jan 7, 2015 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
> > >> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > >> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
> > >> >> +             cpu at 00f {
> > >> >> +                     device_type = "cpu";
> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> > >> >> +                     reg = <0x0 0x00f>;
> > >> >> +                     enable-method = "psci";
> > >> >> +                     arm,associativity = <0 0 0x00f>;
> > >> >> +             };
> > >> >> +             cpu at 100 {
> > >> >> +                     device_type = "cpu";
> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> > >> >> +                     reg = <0x0 0x100>;
> > >> >> +                     enable-method = "psci";
> > >> >> +                     arm,associativity = <0 0 0x100>;
> > >> >> +             };
> > >> >
> > >> > What is the 0x100 offset in the last-level topology field? Does this have
> > >> > no significance to topology at all? I would expect that to be something
> > >> > like cluster number that is relevant to caching and should be represented
> > >> > as a separate level.
> > >>
> > >> i did not understand, can you please explain little more about "
> > >> should be represented as a separate level."
> > >> at present, i have put the hwid of a cpu.
> > >
> > > From what I undertand, the hwid of the CPU contains the "cluster" number in
> > > this bit position, so you typically have a shared L2 or L3 cache between
> > > all cores within a cluster, but separate caches in other clusters.
> > >
> > > If this is the case, there will be a measurable difference in performance
> > > between two processes sharing memory when running on the same cluster,
> > > or when running on different clusters on the same socket. If the
> > > performance difference is relevant, it should be described as a separate
> > > level in the associativity property.
> > you mean, the associativity as array of  <board> <socket> <cluster>
> 
> No, that would leave out the core number, which is required to identify
> the individual thread. I meant adding an extra level such as
> 
> <board> <socket> <cluster> <core>
> 
> A lot of machines will leave out the <board> number because they are
> built with SoCs that don't have a long-distance coherency protocol.

Can't we use phandles to cpu-map nodes instead of a list of numbers (and
yet another topology binding description) ?

Is arm,associativity used solely to map "devices" (inclusive of caches)
to a set of cpus ?

cpu-map misses a notion of distance between hierarchy layers, but we can
add to that.

Lorenzo

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Ganapatrao Kulkarni]

Hi Lorenzo,

On Wed, Jan 14, 2015 at 11:06 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Jan 07, 2015 at 08:18:50AM +0000, Arnd Bergmann wrote:
>> On Wednesday 07 January 2015 12:37:51 Ganapatrao Kulkarni wrote:
>> > Hi Arnd,
>> >
>> > On Wed, Jan 7, 2015 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > > On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
>> > >> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > >> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
>> > >> >> +             cpu at 00f {
>> > >> >> +                     device_type = "cpu";
>> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
>> > >> >> +                     reg = <0x0 0x00f>;
>> > >> >> +                     enable-method = "psci";
>> > >> >> +                     arm,associativity = <0 0 0x00f>;
>> > >> >> +             };
>> > >> >> +             cpu at 100 {
>> > >> >> +                     device_type = "cpu";
>> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
>> > >> >> +                     reg = <0x0 0x100>;
>> > >> >> +                     enable-method = "psci";
>> > >> >> +                     arm,associativity = <0 0 0x100>;
>> > >> >> +             };
>> > >> >
>> > >> > What is the 0x100 offset in the last-level topology field? Does this have
>> > >> > no significance to topology at all? I would expect that to be something
>> > >> > like cluster number that is relevant to caching and should be represented
>> > >> > as a separate level.
>> > >>
>> > >> i did not understand, can you please explain little more about "
>> > >> should be represented as a separate level."
>> > >> at present, i have put the hwid of a cpu.
>> > >
>> > > From what I undertand, the hwid of the CPU contains the "cluster" number in
>> > > this bit position, so you typically have a shared L2 or L3 cache between
>> > > all cores within a cluster, but separate caches in other clusters.
>> > >
>> > > If this is the case, there will be a measurable difference in performance
>> > > between two processes sharing memory when running on the same cluster,
>> > > or when running on different clusters on the same socket. If the
>> > > performance difference is relevant, it should be described as a separate
>> > > level in the associativity property.
>> > you mean, the associativity as array of  <board> <socket> <cluster>
>>
>> No, that would leave out the core number, which is required to identify
>> the individual thread. I meant adding an extra level such as
>>
>> <board> <socket> <cluster> <core>
>>
>> A lot of machines will leave out the <board> number because they are
>> built with SoCs that don't have a long-distance coherency protocol.
>
> Can't we use phandles to cpu-map nodes instead of a list of numbers (and
> yet another topology binding description) ?
cpu-map describes only a cpu topology.
infact, i have tried initially(in v1 patch set) to use topology for
the numa mapping.
However, for numa, we need to define association of cpu, memory and IOs.
arm,associativity is a generic node property and can be used in any dt nodes.
>
> Is arm,associativity used solely to map "devices" (inclusive of caches)
> to a set of cpus ?
>
> cpu-map misses a notion of distance between hierarchy layers, but we can
> add to that.
>
> Lorenzo
thanks
ganapat

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Lorenzo Pieralisi]

On Wed, Jan 14, 2015 at 06:48:32PM +0000, Ganapatrao Kulkarni wrote:
> Hi Lorenzo,
> 
> On Wed, Jan 14, 2015 at 11:06 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Wed, Jan 07, 2015 at 08:18:50AM +0000, Arnd Bergmann wrote:
> >> On Wednesday 07 January 2015 12:37:51 Ganapatrao Kulkarni wrote:
> >> > Hi Arnd,
> >> >
> >> > On Wed, Jan 7, 2015 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > > On Tuesday 06 January 2015 15:04:26 Ganapatrao Kulkarni wrote:
> >> > >> On Sat, Jan 3, 2015 at 2:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > >> > On Wednesday 31 December 2014 13:03:27 Ganapatrao Kulkarni wrote:
> >> > >> >> +             cpu at 00f {
> >> > >> >> +                     device_type = "cpu";
> >> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> > >> >> +                     reg = <0x0 0x00f>;
> >> > >> >> +                     enable-method = "psci";
> >> > >> >> +                     arm,associativity = <0 0 0x00f>;
> >> > >> >> +             };
> >> > >> >> +             cpu at 100 {
> >> > >> >> +                     device_type = "cpu";
> >> > >> >> +                     compatible = "cavium,thunder", "arm,armv8";
> >> > >> >> +                     reg = <0x0 0x100>;
> >> > >> >> +                     enable-method = "psci";
> >> > >> >> +                     arm,associativity = <0 0 0x100>;
> >> > >> >> +             };
> >> > >> >
> >> > >> > What is the 0x100 offset in the last-level topology field? Does this have
> >> > >> > no significance to topology at all? I would expect that to be something
> >> > >> > like cluster number that is relevant to caching and should be represented
> >> > >> > as a separate level.
> >> > >>
> >> > >> i did not understand, can you please explain little more about "
> >> > >> should be represented as a separate level."
> >> > >> at present, i have put the hwid of a cpu.
> >> > >
> >> > > From what I undertand, the hwid of the CPU contains the "cluster" number in
> >> > > this bit position, so you typically have a shared L2 or L3 cache between
> >> > > all cores within a cluster, but separate caches in other clusters.
> >> > >
> >> > > If this is the case, there will be a measurable difference in performance
> >> > > between two processes sharing memory when running on the same cluster,
> >> > > or when running on different clusters on the same socket. If the
> >> > > performance difference is relevant, it should be described as a separate
> >> > > level in the associativity property.
> >> > you mean, the associativity as array of  <board> <socket> <cluster>
> >>
> >> No, that would leave out the core number, which is required to identify
> >> the individual thread. I meant adding an extra level such as
> >>
> >> <board> <socket> <cluster> <core>
> >>
> >> A lot of machines will leave out the <board> number because they are
> >> built with SoCs that don't have a long-distance coherency protocol.
> >
> > Can't we use phandles to cpu-map nodes instead of a list of numbers (and
> > yet another topology binding description) ?
> cpu-map describes only a cpu topology.
> infact, i have tried initially(in v1 patch set) to use topology for
> the numa mapping.
> However, for numa, we need to define association of cpu, memory and IOs.
> arm,associativity is a generic node property and can be used in any dt nodes.

I understand that, I was advising to define "arm,associativity" as a
phandle in cpu nodes AND all devices.

Why can't you make it point at a phandle in the cpu-map instead of adding
a t-uple doing the same thing. Am I missing something here ?
cpu-map allows you to describe the system hierarchy and can expand beyond
clusters (several layers of clusterings, above core it is just a way to
define the system hierarchy, leaves node will always be cores or threads).

On a side note, one of the reasons cpu-map was devised for was exactly
that, to allow mappings of resources (ie IRQs but it is valid for caches
and other devices too) to groups of CPUs.

Is there anything that you can't do by using cpu-map phandles to
describe devices associativity ?

We have to add bindings that allow to compute the distance as you
do by using the reference points (I am reading the code to figure
out how it is used), but that's feasible as a binding update.

Lorenzo

[RFC PATCH v3 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.

[Arnd Bergmann]

On Wednesday 14 January 2015 23:49:05 Lorenzo Pieralisi wrote:
> On Wed, Jan 14, 2015 at 06:48:32PM +0000, Ganapatrao Kulkarni wrote:
> > On Wed, Jan 14, 2015 at 11:06 PM, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > > On Wed, Jan 07, 2015 at 08:18:50AM +0000, Arnd Bergmann wrote:
> > >> No, that would leave out the core number, which is required to identify
> > >> the individual thread. I meant adding an extra level such as
> > >>
> > >> <board> <socket> <cluster> <core>
> > >>
> > >> A lot of machines will leave out the <board> number because they are
> > >> built with SoCs that don't have a long-distance coherency protocol.
> > >
> > > Can't we use phandles to cpu-map nodes instead of a list of numbers (and
> > > yet another topology binding description) ?
> > cpu-map describes only a cpu topology.
> > infact, i have tried initially(in v1 patch set) to use topology for
> > the numa mapping.
> > However, for numa, we need to define association of cpu, memory and IOs.
> > arm,associativity is a generic node property and can be used in any dt nodes.
> 
> I understand that, I was advising to define "arm,associativity" as a
> phandle in cpu nodes AND all devices.
> 
> Why can't you make it point at a phandle in the cpu-map instead of adding
> a t-uple doing the same thing. Am I missing something here ?

Most importantly, it's following an existing spec for ibm,associativity,
which defines topology in terms of associativity, not a hierarchical tree.

> cpu-map allows you to describe the system hierarchy and can expand beyond
> clusters (several layers of clusterings, above core it is just a way to
> define the system hierarchy, leaves node will always be cores or threads).

> On a side note, one of the reasons cpu-map was devised for was exactly
> that, to allow mappings of resources (ie IRQs but it is valid for caches
> and other devices too) to groups of CPUs.
> 
> Is there anything that you can't do by using cpu-map phandles to
> describe devices associativity ?

- It doesn't work for cpu-less nodes.
- It fails if you have multiple paths between two devices, rather than
  a strict tree.
- It doesn't (yet) have a way to define which levels are relevant to NUMA
  topology.
- the phandle references are done in the wrong way if you want to
  represent a lot of devices.

> We have to add bindings that allow to compute the distance as you
> do by using the reference points (I am reading the code to figure
> out how it is used), but that's feasible as a binding update.

It's very unfortunate that we have two conflicting bindings that are
established. I still think that the associativity binding is more
flexible, but we could try to extend the arm topology binding if
necessary, but I'm not sure the end result of that would be better.

	Arnd