| Message ID | 1366302046.3388.8.camel@linaro1.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 18 Apr 2013, Jon Medhurst (Tixy) wrote: > On resume from CPU power down any trace hooks enabled in cpu_init() > will get called before that function has done set_my_cpu_offset(), > so any use of per-cpu variables by trace hook code will cause bad > things to happen. Prevent this by marking the function notrace. > > This fixes lockups/crashes seen when enabling function tracer on TC2 > with the not yet mainlined cpuidle driver. This has potential to fix things on other platforms too. > > Signed-off-by: Jon Medhurst <tixy@linaro.org> Acked-by: Nicolas Pitre <nico@linaro.org> I think you may send it to RMK's patch system. > --- > arch/arm/kernel/setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > index d343a6c..943cbf0 100644 > --- a/arch/arm/kernel/setup.c > +++ b/arch/arm/kernel/setup.c > @@ -390,7 +390,7 @@ static void __init feat_v6_fixup(void) > * > * cpu_init sets up the per-CPU stacks. > */ > -void cpu_init(void) > +void notrace cpu_init(void) > { > unsigned int cpu = smp_processor_id(); > struct stack *stk = &stacks[cpu]; > -- > 1.7.10.4 > >
On Thu, Apr 18, 2013 at 05:20:46PM +0100, Jon Medhurst (Tixy) wrote: > On resume from CPU power down any trace hooks enabled in cpu_init() > will get called before that function has done set_my_cpu_offset(), > so any use of per-cpu variables by trace hook code will cause bad > things to happen. Prevent this by marking the function notrace. > > This fixes lockups/crashes seen when enabling function tracer on TC2 > with the not yet mainlined cpuidle driver. Looks sane. Needs to go to the patch system though. Not sure if it'll make 3.9 given its proximity (we're on -rc8).
On Mon, 2013-04-22 at 15:07 +0100, Russell King - ARM Linux wrote: > On Thu, Apr 18, 2013 at 05:20:46PM +0100, Jon Medhurst (Tixy) wrote: > > On resume from CPU power down any trace hooks enabled in cpu_init() > > will get called before that function has done set_my_cpu_offset(), > > so any use of per-cpu variables by trace hook code will cause bad > > things to happen. Prevent this by marking the function notrace. > > > > This fixes lockups/crashes seen when enabling function tracer on TC2 > > with the not yet mainlined cpuidle driver. > > Looks sane. Needs to go to the patch system though. Not sure if it'll > make 3.9 given its proximity (we're on -rc8). Added as patch 7700/1. Don't think there is a big rush as the problem has probably been there since 3.8 when commit 14318efb (implement optimized percpu variable access) started using TPIDRPRW; which is presumably lost over CPU power down.
On Mon, Apr 22, 2013 at 07:08:36PM +0100, Jon Medhurst (Tixy) wrote: > On Mon, 2013-04-22 at 15:07 +0100, Russell King - ARM Linux wrote: > > On Thu, Apr 18, 2013 at 05:20:46PM +0100, Jon Medhurst (Tixy) wrote: > > > On resume from CPU power down any trace hooks enabled in cpu_init() > > > will get called before that function has done set_my_cpu_offset(), > > > so any use of per-cpu variables by trace hook code will cause bad > > > things to happen. Prevent this by marking the function notrace. > > > > > > This fixes lockups/crashes seen when enabling function tracer on TC2 > > > with the not yet mainlined cpuidle driver. > > > > Looks sane. Needs to go to the patch system though. Not sure if it'll > > make 3.9 given its proximity (we're on -rc8). > > Added as patch 7700/1. Don't think there is a big rush as the problem > has probably been there since 3.8 when commit 14318efb (implement > optimized percpu variable access) started using TPIDRPRW; which is > presumably lost over CPU power down. $ pdb getpatch 7700/1 |tr ' ' '_' |less diff_--git_a/arch/arm/kernel/setup.c_b/arch/arm/kernel/setup.c index_d343a6c..943cbf0_100644 ---_a/arch/arm/kernel/setup.c +++_b/arch/arm/kernel/setup.c @@_-390,7_+390,7_@@_static_void___init_feat_v6_fixup(void) __* __*_cpu_init_sets_up_the_per-CPU_stacks. __*/ -void_cpu_init(void) +void_notrace_cpu_init(void) _{ ________unsigned_int_cpu_=_smp_processor_id(); ________struct_stack_*stk_=_&stacks[cpu]; Can you guess why git has a problem appying the above... you seem to have sent the patch from your mailer, I guess your mailer thinks it knows better when it comes to what characters should be in your message... or maybe its a cut'n'paste issue?
On Thu, 2013-04-25 at 13:15 +0100, Russell King - ARM Linux wrote: > $ pdb getpatch 7700/1 |tr ' ' '_' |less > diff_--git_a/arch/arm/kernel/setup.c_b/arch/arm/kernel/setup.c > index_d343a6c..943cbf0_100644 > ---_a/arch/arm/kernel/setup.c > +++_b/arch/arm/kernel/setup.c > @@_-390,7_+390,7_@@_static_void___init_feat_v6_fixup(void) > __* > __*_cpu_init_sets_up_the_per-CPU_stacks. > __*/ > -void_cpu_init(void) > +void_notrace_cpu_init(void) > _{ > ________unsigned_int_cpu_=_smp_processor_id(); > ________struct_stack_*stk_=_&stacks[cpu]; > > Can you guess why git has a problem appying the above... Sorry about that, the user forgot to tell their mailer it was pre-formatted text. I've sent a fixed version. Note to self: bcc self on patch submissions and then run them though 'git am' to catch problems before others hit them.
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index d343a6c..943cbf0 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -390,7 +390,7 @@ static void __init feat_v6_fixup(void) * * cpu_init sets up the per-CPU stacks. */ -void cpu_init(void) +void notrace cpu_init(void) { unsigned int cpu = smp_processor_id(); struct stack *stk = &stacks[cpu];
On resume from CPU power down any trace hooks enabled in cpu_init() will get called before that function has done set_my_cpu_offset(), so any use of per-cpu variables by trace hook code will cause bad things to happen. Prevent this by marking the function notrace. This fixes lockups/crashes seen when enabling function tracer on TC2 with the not yet mainlined cpuidle driver. Signed-off-by: Jon Medhurst <tixy@linaro.org> --- arch/arm/kernel/setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)