2015-06-10 04:50:44

by Nikunj A Dadhania

[permalink] [raw]
Subject: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path

Node distance will not get initialized in certain cases as
dynamic-reconfiguration path does not initialize the lookup table.

There is an assumption about the associativity list in
initialize_distance_lookup_table(). Associativity list has two forms:

a) [cpu,memory]@x/ibm,associativity has following
format:

<N> <N integers>

b) ibm,dynamic-reconfiguration-memory/ibm,associativity-lookup-arrays

<M> <N> <M*N integers>
M = the number of associativity lists
N = the number of entries per associativity list

Fix initialize_distance_lookup_table() so that it does not assume
"case a". And update the caller to skip the length field before
sending the associativity list.

Call initialize_distance_lookup_table() from drconf path with
appropriate associativity list.

Reported-by: Bharata B Rao <[email protected]>
Signed-off-by: Nikunj A Dadhania <[email protected]>
---
arch/powerpc/mm/numa.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 5e80621..8b9502a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -225,7 +225,7 @@ static void initialize_distance_lookup_table(int nid,
for (i = 0; i < distance_ref_points_depth; i++) {
const __be32 *entry;

- entry = &associativity[be32_to_cpu(distance_ref_points[i])];
+ entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
distance_lookup_table[nid][i] = of_read_number(entry, 1);
}
}
@@ -248,8 +248,12 @@ static int associativity_to_nid(const __be32 *associativity)
nid = -1;

if (nid > 0 &&
- of_read_number(associativity, 1) >= distance_ref_points_depth)
- initialize_distance_lookup_table(nid, associativity);
+ of_read_number(associativity, 1) >= distance_ref_points_depth) {
+ /*
+ * Skip the length field and send start of associativity array
+ */
+ initialize_distance_lookup_table(nid, associativity + 1);
+ }

out:
return nid;
@@ -507,6 +511,12 @@ static int of_drconf_to_nid_single(struct of_drconf_cell *drmem,

if (nid == 0xffff || nid >= MAX_NUMNODES)
nid = default_nid;
+
+ if (nid > 0) {
+ index = drmem->aa_index * aa->array_sz;
+ initialize_distance_lookup_table(nid,
+ &aa->arrays[index]);
+ }
}

return nid;
--
1.8.3.1


2015-06-10 05:08:55

by Nikunj A Dadhania

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path


Reworded commit log:

From: Nikunj A Dadhania <[email protected]>

powerpc/numa: initialize distance lookup table from drconf path

In some situations, a NUMA guest that supports
ibm,dynamic-memory-reconfiguration node will end up having flat NUMA
distances between nodes. This is because of two problems in the
current code.

1) Different representations of associativity lists.

There is an assumption about the associativity list in
initialize_distance_lookup_table(). Associativity list has two forms:

a) [cpu,memory]@x/ibm,associativity has following
format:
<N> <N integers>

b) ibm,dynamic-reconfiguration-memory/ibm,associativity-lookup-arrays

<M> <N> <M associativity lists each having N integers>
M = the number of associativity lists
N = the number of entries per associativity list

Fix initialize_distance_lookup_table() so that it does not assume
"case a". And update the caller to skip the length field before
sending the associativity list.

2) Distance table not getting updated from drconf path.

Node distance table will not get initialized in certain cases as
ibm,dynamic-reconfiguration-memory path does not initialize the
lookup table.

Call initialize_distance_lookup_table() from drconf path with
appropriate associativity list.

Reported-by: Bharata B Rao <[email protected]>
Signed-off-by: Nikunj A Dadhania <[email protected]>
---
arch/powerpc/mm/numa.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 5e80621..8b9502a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -225,7 +225,7 @@ static void initialize_distance_lookup_table(int nid,
for (i = 0; i < distance_ref_points_depth; i++) {
const __be32 *entry;

- entry = &associativity[be32_to_cpu(distance_ref_points[i])];
+ entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
distance_lookup_table[nid][i] = of_read_number(entry, 1);
}
}
@@ -248,8 +248,12 @@ static int associativity_to_nid(const __be32 *associativity)
nid = -1;

if (nid > 0 &&
- of_read_number(associativity, 1) >= distance_ref_points_depth)
- initialize_distance_lookup_table(nid, associativity);
+ of_read_number(associativity, 1) >= distance_ref_points_depth) {
+ /*
+ * Skip the length field and send start of associativity array
+ */
+ initialize_distance_lookup_table(nid, associativity + 1);
+ }

out:
return nid;
@@ -507,6 +511,12 @@ static int of_drconf_to_nid_single(struct of_drconf_cell *drmem,

if (nid == 0xffff || nid >= MAX_NUMNODES)
nid = default_nid;
+
+ if (nid > 0) {
+ index = drmem->aa_index * aa->array_sz;
+ initialize_distance_lookup_table(nid,
+ &aa->arrays[index]);
+ }
}

return nid;
--
1.8.3.1

2015-06-16 04:06:04

by Nikunj A Dadhania

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path

Nikunj A Dadhania <[email protected]> writes:

> Reworded commit log:
>
> From: Nikunj A Dadhania <[email protected]>
>
> powerpc/numa: initialize distance lookup table from drconf path
>

Ping ?

Regards
Nikunj

2015-06-23 01:16:59

by Anton Blanchard

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path

Hi Nikunj,

> From: Nikunj A Dadhania <[email protected]>
>
> powerpc/numa: initialize distance lookup table from drconf path
>
> In some situations, a NUMA guest that supports
> ibm,dynamic-memory-reconfiguration node will end up having flat NUMA
> distances between nodes. This is because of two problems in the
> current code.

Thanks for the patch. Have we tested that this doesn't regress the
non dynamic representation?

Regards,
Anton

> 1) Different representations of associativity lists.
>
> There is an assumption about the associativity list in
> initialize_distance_lookup_table(). Associativity list has two
> forms:
>
> a) [cpu,memory]@x/ibm,associativity has following
> format:
> <N> <N integers>
>
> b)
> ibm,dynamic-reconfiguration-memory/ibm,associativity-lookup-arrays
>
> <M> <N> <M associativity lists each having N integers>
> M = the number of associativity lists
> N = the number of entries per associativity list
>
> Fix initialize_distance_lookup_table() so that it does not assume
> "case a". And update the caller to skip the length field before
> sending the associativity list.
>
> 2) Distance table not getting updated from drconf path.
>
> Node distance table will not get initialized in certain cases as
> ibm,dynamic-reconfiguration-memory path does not initialize the
> lookup table.
>
> Call initialize_distance_lookup_table() from drconf path with
> appropriate associativity list.
>
> Reported-by: Bharata B Rao <[email protected]>
> Signed-off-by: Nikunj A Dadhania <[email protected]>
> ---
> arch/powerpc/mm/numa.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 5e80621..8b9502a 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -225,7 +225,7 @@ static void initialize_distance_lookup_table(int
> nid, for (i = 0; i < distance_ref_points_depth; i++) {
> const __be32 *entry;
>
> - entry =
> &associativity[be32_to_cpu(distance_ref_points[i])];
> + entry =
> &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
> distance_lookup_table[nid][i] = of_read_number(entry, 1); }
> }
> @@ -248,8 +248,12 @@ static int associativity_to_nid(const __be32
> *associativity) nid = -1;
>
> if (nid > 0 &&
> - of_read_number(associativity, 1) >=
> distance_ref_points_depth)
> - initialize_distance_lookup_table(nid, associativity);
> + of_read_number(associativity, 1) >=
> distance_ref_points_depth) {
> + /*
> + * Skip the length field and send start of
> associativity array
> + */
> + initialize_distance_lookup_table(nid, associativity
> + 1);
> + }
>
> out:
> return nid;
> @@ -507,6 +511,12 @@ static int of_drconf_to_nid_single(struct
> of_drconf_cell *drmem,
> if (nid == 0xffff || nid >= MAX_NUMNODES)
> nid = default_nid;
> +
> + if (nid > 0) {
> + index = drmem->aa_index * aa->array_sz;
> + initialize_distance_lookup_table(nid,
> +
> &aa->arrays[index]);
> + }
> }
>
> return nid;

2015-06-23 07:03:41

by Nikunj A Dadhania

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path


Hi Anton,

Anton Blanchard <[email protected]> writes:
> Hi Nikunj,
>
>> From: Nikunj A Dadhania <[email protected]>
>>
>> powerpc/numa: initialize distance lookup table from drconf path
>>
>> In some situations, a NUMA guest that supports
>> ibm,dynamic-memory-reconfiguration node will end up having flat NUMA
>> distances between nodes. This is because of two problems in the
>> current code.
>
> Thanks for the patch. Have we tested that this doesn't regress the
> non dynamic representation?

Yes, that is tested. And works as expected.

Regards
Nikunj

2015-06-26 06:46:45

by Nikunj A Dadhania

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path


Hi Anton/Michael,

Nikunj A Dadhania <[email protected]> writes:
> Hi Anton,
>
> Anton Blanchard <[email protected]> writes:
>> Hi Nikunj,
>>
>>> From: Nikunj A Dadhania <[email protected]>
>>>
>>> powerpc/numa: initialize distance lookup table from drconf path
>>>
>>> In some situations, a NUMA guest that supports
>>> ibm,dynamic-memory-reconfiguration node will end up having flat NUMA
>>> distances between nodes. This is because of two problems in the
>>> current code.
>>
>> Thanks for the patch. Have we tested that this doesn't regress the
>> non dynamic representation?
>
> Yes, that is tested. And works as expected.

If the patch looks fine, can this be pushed upstream ?

Regards,
Nikunj

2015-07-01 22:31:25

by Anton Blanchard

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path

Hi Nikunj,

> > Thanks for the patch. Have we tested that this doesn't regress the
> > non dynamic representation?
>
> Yes, that is tested. And works as expected.

Great, you can add:

Acked-by: Anton Blanchard <[email protected]>

Anton

2015-07-02 03:44:41

by Michael Ellerman

[permalink] [raw]
Subject: Re: [RFC PATCH] powerpc/numa: initialize distance lookup table from drconf path

On Thu, 2015-07-02 at 08:31 +1000, Anton Blanchard wrote:
> Hi Nikunj,
>
> > > Thanks for the patch. Have we tested that this doesn't regress the
> > > non dynamic representation?
> >
> > Yes, that is tested. And works as expected.
>
> Great, you can add:
>
> Acked-by: Anton Blanchard <[email protected]>

Hi Nikunj,

Please resend a non-RFC version, with the updated changelog and Anton's ack
included.

cheers