* [PATCH] Correct cpu affinity for dlpar added cpus
@ 2015-04-28 15:37 Nathan Fontenot
2015-04-29 4:53 ` Michael Ellerman
0 siblings, 1 reply; 3+ messages in thread
From: Nathan Fontenot @ 2015-04-28 15:37 UTC (permalink / raw)
To: linuxppc-dev
The incorrect ordering of operations during cpu dlpar causes the affinity
of cpus being added to be invalid. Phyp does not assign affinity information
for a cpu until the rtas set-indicator calls are made to set the isolation
and allocation state. In the current code we call rtas configure-connector
before making the set-indicator calls which results in invalid data in the
ibm,associativity property for the cpu we're adding.
This patch corrects the order of operations to make the set-indicator
calls (done in acquire_drc) before calling configure-connector.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index b4b1109..019d34a 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -412,6 +412,10 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
if (rc)
return -EINVAL;
+ rc = dlpar_acquire_drc(drc_index);
+ if (rc)
+ return -EINVAL;
+
parent = of_find_node_by_path("/cpus");
if (!parent)
return -ENODEV;
@@ -422,12 +426,6 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
of_node_put(parent);
- rc = dlpar_acquire_drc(drc_index);
- if (rc) {
- dlpar_free_cc_nodes(dn);
- return -EINVAL;
- }
-
rc = dlpar_attach_node(dn);
if (rc) {
dlpar_release_drc(drc_index);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Correct cpu affinity for dlpar added cpus
2015-04-28 15:37 [PATCH] Correct cpu affinity for dlpar added cpus Nathan Fontenot
@ 2015-04-29 4:53 ` Michael Ellerman
2015-04-29 15:33 ` Nathan Fontenot
0 siblings, 1 reply; 3+ messages in thread
From: Michael Ellerman @ 2015-04-29 4:53 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev
Subject should be "powerpc/pseries: ..." please.
On Tue, 2015-04-28 at 10:37 -0500, Nathan Fontenot wrote:
> The incorrect ordering of operations during cpu dlpar causes the affinity
> of cpus being added to be invalid. Phyp does not assign affinity information
> for a cpu until the rtas set-indicator calls are made to set the isolation
> and allocation state. In the current code we call rtas configure-connector
> before making the set-indicator calls which results in invalid data in the
> ibm,associativity property for the cpu we're adding.
Invalid and benign? Or invalid and causes an oops or ..?
> This patch corrects the order of operations to make the set-indicator
> calls (done in acquire_drc) before calling configure-connector.
Which commit added the code and/or caused it to be wrong?
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n187
While looking at the code I notice it looks like we leak a reference if
dlpar_configure_connector() fails:
parent = of_find_node_by_path("/cpus");
if (!parent)
return -ENODEV;
dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
if (!dn)
return -EINVAL;
of_node_put(parent);
Please send a separate patch to fix that.
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Correct cpu affinity for dlpar added cpus
2015-04-29 4:53 ` Michael Ellerman
@ 2015-04-29 15:33 ` Nathan Fontenot
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Fontenot @ 2015-04-29 15:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
On 04/28/2015 11:53 PM, Michael Ellerman wrote:
> Subject should be "powerpc/pseries: ..." please.
ok
>
> On Tue, 2015-04-28 at 10:37 -0500, Nathan Fontenot wrote:
>> The incorrect ordering of operations during cpu dlpar causes the affinity
>> of cpus being added to be invalid. Phyp does not assign affinity information
>> for a cpu until the rtas set-indicator calls are made to set the isolation
>> and allocation state. In the current code we call rtas configure-connector
>> before making the set-indicator calls which results in invalid data in the
>> ibm,associativity property for the cpu we're adding.
>
> Invalid and benign? Or invalid and causes an oops or ..?
>
Invalid and benign. I'll add this to the description.
>> This patch corrects the order of operations to make the set-indicator
>> calls (done in acquire_drc) before calling configure-connector.
>
> Which commit added the code and/or caused it to be wrong?
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n187
>
And the commit that this fixes..
>
> While looking at the code I notice it looks like we leak a reference if
> dlpar_configure_connector() fails:
>
> parent = of_find_node_by_path("/cpus");
> if (!parent)
> return -ENODEV;
>
> dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
> if (!dn)
> return -EINVAL;
>
> of_node_put(parent);
>
>
> Please send a separate patch to fix that.
Yep, saw that too.
New patches on their way.
Thanks for the feedback,
-Nathan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-04-29 15:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-28 15:37 [PATCH] Correct cpu affinity for dlpar added cpus Nathan Fontenot
2015-04-29 4:53 ` Michael Ellerman
2015-04-29 15:33 ` Nathan Fontenot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).