* [PATCH] Do not update sysfs cpu registration from invalid context
@ 2013-06-24 14:14 Nathan Fontenot
2013-06-24 17:18 ` Seth Jennings
2013-06-25 1:50 ` Michael Ellerman
0 siblings, 2 replies; 8+ messages in thread
From: Nathan Fontenot @ 2013-06-24 14:14 UTC (permalink / raw)
To: linuxppc-dev
The topology update code that updates the cpu node registration in sysfs
should not be called while in stop_machine(). The register/unregister
calls take a lock and may sleep.
This patch moves these calls outside of the call to stop_machine().
Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c 2013-06-24 06:53:31.000000000 -0500
+++ powerpc/arch/powerpc/mm/numa.c 2013-06-24 06:56:30.000000000 -0500
@@ -1433,11 +1433,9 @@
if (cpu != update->cpu)
continue;
- unregister_cpu_under_node(update->cpu, update->old_nid);
unmap_cpu_from_node(update->cpu);
map_cpu_to_node(update->cpu, update->new_nid);
vdso_getcpu_init();
- register_cpu_under_node(update->cpu, update->new_nid);
}
return 0;
@@ -1485,6 +1483,9 @@
stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
for (ud = &updates[0]; ud; ud = ud->next) {
+ unregister_cpu_under_node(update->cpu, update->old_nid);
+ register_cpu_under_node(update->cpu, update->new_nid);
+
dev = get_cpu_device(ud->cpu);
if (dev)
kobject_uevent(&dev->kobj, KOBJ_CHANGE);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-24 14:14 [PATCH] Do not update sysfs cpu registration from invalid context Nathan Fontenot
@ 2013-06-24 17:18 ` Seth Jennings
2013-06-24 19:16 ` Seth Jennings
2013-06-25 1:50 ` Michael Ellerman
1 sibling, 1 reply; 8+ messages in thread
From: Seth Jennings @ 2013-06-24 17:18 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev
On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
> The topology update code that updates the cpu node registration in sysfs
> should not be called while in stop_machine(). The register/unregister
> calls take a lock and may sleep.
>
> This patch moves these calls outside of the call to stop_machine().
>
> Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
Reviewed-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Index: powerpc/arch/powerpc/mm/numa.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/mm/numa.c 2013-06-24 06:53:31.000000000 -0500
> +++ powerpc/arch/powerpc/mm/numa.c 2013-06-24 06:56:30.000000000 -0500
> @@ -1433,11 +1433,9 @@
> if (cpu != update->cpu)
> continue;
>
> - unregister_cpu_under_node(update->cpu, update->old_nid);
> unmap_cpu_from_node(update->cpu);
> map_cpu_to_node(update->cpu, update->new_nid);
> vdso_getcpu_init();
> - register_cpu_under_node(update->cpu, update->new_nid);
> }
>
> return 0;
> @@ -1485,6 +1483,9 @@
> stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>
> for (ud = &updates[0]; ud; ud = ud->next) {
> + unregister_cpu_under_node(update->cpu, update->old_nid);
> + register_cpu_under_node(update->cpu, update->new_nid);
> +
> dev = get_cpu_device(ud->cpu);
> if (dev)
> kobject_uevent(&dev->kobj, KOBJ_CHANGE);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-24 17:18 ` Seth Jennings
@ 2013-06-24 19:16 ` Seth Jennings
2013-06-24 19:25 ` Nathan Fontenot
0 siblings, 1 reply; 8+ messages in thread
From: Seth Jennings @ 2013-06-24 19:16 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev
On Mon, Jun 24, 2013 at 12:18:04PM -0500, Seth Jennings wrote:
> On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
> > The topology update code that updates the cpu node registration in sysfs
> > should not be called while in stop_machine(). The register/unregister
> > calls take a lock and may sleep.
> >
> > This patch moves these calls outside of the call to stop_machine().
> >
> > Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
>
> Reviewed-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
Gah! I _knew_ I should have waited for my cross compiler to finish
building. This thing doesn't build:
CC arch/powerpc/mm/numa.o
/home/sjennings/ltc/linux/arch/powerpc/mm/numa.c: In function 'arch_update_cpu_topology':
/home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: 'update' undeclared (first use in this function)
/home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: (Each undeclared identifier is reported only once
/home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: for each function it appears in.)
s/update/ud/ in the *_cpu_under_node() calls.
Seth
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-24 19:16 ` Seth Jennings
@ 2013-06-24 19:25 ` Nathan Fontenot
2013-06-25 1:50 ` Michael Ellerman
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Fontenot @ 2013-06-24 19:25 UTC (permalink / raw)
To: Seth Jennings; +Cc: linuxppc-dev
On 06/24/2013 02:16 PM, Seth Jennings wrote:
> On Mon, Jun 24, 2013 at 12:18:04PM -0500, Seth Jennings wrote:
>> On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
>>> The topology update code that updates the cpu node registration in sysfs
>>> should not be called while in stop_machine(). The register/unregister
>>> calls take a lock and may sleep.
>>>
>>> This patch moves these calls outside of the call to stop_machine().
>>>
>>> Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>
>> Reviewed-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>
> Gah! I _knew_ I should have waited for my cross compiler to finish
> building. This thing doesn't build:
>
> CC arch/powerpc/mm/numa.o
> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c: In function 'arch_update_cpu_topology':
> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: 'update' undeclared (first use in this function)
> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: (Each undeclared identifier is reported only once
> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: for each function it appears in.)
>
> s/update/ud/ in the *_cpu_under_node() calls.
Oops! Time for patch submission re-education training.
New, and correct, patch coming soon.
-Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-24 19:25 ` Nathan Fontenot
@ 2013-06-25 1:50 ` Michael Ellerman
2013-06-25 2:46 ` Nathan Fontenot
0 siblings, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2013-06-25 1:50 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: Seth Jennings, linuxppc-dev
On Mon, Jun 24, 2013 at 02:25:59PM -0500, Nathan Fontenot wrote:
> On 06/24/2013 02:16 PM, Seth Jennings wrote:
> > On Mon, Jun 24, 2013 at 12:18:04PM -0500, Seth Jennings wrote:
> >> On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
> >>> The topology update code that updates the cpu node registration in sysfs
> >>> should not be called while in stop_machine(). The register/unregister
> >>> calls take a lock and may sleep.
> >>>
> >>> This patch moves these calls outside of the call to stop_machine().
> >>>
> >>> Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
> >>
> >> Reviewed-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> >
> > Gah! I _knew_ I should have waited for my cross compiler to finish
> > building. This thing doesn't build:
> >
> > CC arch/powerpc/mm/numa.o
> > /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c: In function 'arch_update_cpu_topology':
> > /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: 'update' undeclared (first use in this function)
> > /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: (Each undeclared identifier is reported only once
> > /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: for each function it appears in.)
> >
> > s/update/ud/ in the *_cpu_under_node() calls.
>
> Oops! Time for patch submission re-education training.
We've all done it, but yes :)
I try to stick to:
1. write code.
2. build code.
3. test code.
4. submit code.
I imagine you tested an early version of the patch, or on RHEL or
something, but that can bite you like this. Whenever possible you should
build & test the exact code you submit, though that can be hard when
trees are moving quickly underneath you.
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-24 14:14 [PATCH] Do not update sysfs cpu registration from invalid context Nathan Fontenot
2013-06-24 17:18 ` Seth Jennings
@ 2013-06-25 1:50 ` Michael Ellerman
2013-06-25 2:47 ` Nathan Fontenot
1 sibling, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2013-06-25 1:50 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev
On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
> The topology update code that updates the cpu node registration in sysfs
> should not be called while in stop_machine(). The register/unregister
> calls take a lock and may sleep.
>
> This patch moves these calls outside of the call to stop_machine().
What happens? Do we lockup or do you just get a warning?
And what commit introduced the breakage?
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-25 1:50 ` Michael Ellerman
@ 2013-06-25 2:46 ` Nathan Fontenot
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Fontenot @ 2013-06-25 2:46 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Seth Jennings, linuxppc-dev
On 06/24/2013 08:50 PM, Michael Ellerman wrote:
> On Mon, Jun 24, 2013 at 02:25:59PM -0500, Nathan Fontenot wrote:
>> On 06/24/2013 02:16 PM, Seth Jennings wrote:
>>> On Mon, Jun 24, 2013 at 12:18:04PM -0500, Seth Jennings wrote:
>>>> On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
>>>>> The topology update code that updates the cpu node registration in sysfs
>>>>> should not be called while in stop_machine(). The register/unregister
>>>>> calls take a lock and may sleep.
>>>>>
>>>>> This patch moves these calls outside of the call to stop_machine().
>>>>>
>>>>> Signed-off-by:Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>>>
>>>> Reviewed-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>>
>>> Gah! I _knew_ I should have waited for my cross compiler to finish
>>> building. This thing doesn't build:
>>>
>>> CC arch/powerpc/mm/numa.o
>>> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c: In function 'arch_update_cpu_topology':
>>> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: 'update' undeclared (first use in this function)
>>> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: (Each undeclared identifier is reported only once
>>> /home/sjennings/ltc/linux/arch/powerpc/mm/numa.c:1486: error: for each function it appears in.)
>>>
>>> s/update/ud/ in the *_cpu_under_node() calls.
>>
>> Oops! Time for patch submission re-education training.
>
> We've all done it, but yes :)
>
> I try to stick to:
>
> 1. write code.
I would suggest
1a. ensure you have the proper config options set
> 2. build code.
> 3. test code.
> 4. submit code.
>
> I imagine you tested an early version of the patch, or on RHEL or
> something, but that can bite you like this. Whenever possible you should
> build & test the exact code you submit, though that can be hard when
> trees are moving quickly underneath you.
Yep, bitten by 1a. I didn't verify the config options I was building
with and had SMP disabled in the tree. This ifdef'ed out my code.
-Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not update sysfs cpu registration from invalid context
2013-06-25 1:50 ` Michael Ellerman
@ 2013-06-25 2:47 ` Nathan Fontenot
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Fontenot @ 2013-06-25 2:47 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
On 06/24/2013 08:50 PM, Michael Ellerman wrote:
> On Mon, Jun 24, 2013 at 09:14:23AM -0500, Nathan Fontenot wrote:
>> The topology update code that updates the cpu node registration in sysfs
>> should not be called while in stop_machine(). The register/unregister
>> calls take a lock and may sleep.
>>
>> This patch moves these calls outside of the call to stop_machine().
>
> What happens? Do we lockup or do you just get a warning?
>
> And what commit introduced the breakage?
Guilty on on all counts. Hopefully I can still get by with stern warning.
-Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-06-25 2:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-24 14:14 [PATCH] Do not update sysfs cpu registration from invalid context Nathan Fontenot
2013-06-24 17:18 ` Seth Jennings
2013-06-24 19:16 ` Seth Jennings
2013-06-24 19:25 ` Nathan Fontenot
2013-06-25 1:50 ` Michael Ellerman
2013-06-25 2:46 ` Nathan Fontenot
2013-06-25 1:50 ` Michael Ellerman
2013-06-25 2:47 ` Nathan Fontenot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.