[PATCH v2] tracing/ftrace: don't trace on early stage of a secondary cpu boot

[PATCH v2] tracing/ftrace: don't trace on early stage of a secondary cpu boot

[Frederic Weisbecker]

Impact: fix a crash/hard-reboot while enabling cpu on runtime

On some archs, the boot of a secondary cpu can have an early fragile state.
On x86-64, the pda is not initialized on the first stage of a cpu boot but
it is needed to get the cpu number and the current task pointer. These datas
are needed during tracing. As they were dereferenced at this stage, we got a
crash while turning on a cpu on runtime while tracing.

Some other archs like ia64 can have such kind of issue too.

Changes on v2:

We drop the previous solution of a per-arch called function to guess the current state
of a cpu. That could make slow the tracing.
This patch just drop the -pg flag on arch/x86/kernel/cpu/common.c where
live the low level cpu boot functions, and on start_secondary() and a helper
function used at this stage.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/include/asm/msr.h   |    3 ++-
 arch/x86/kernel/cpu/Makefile |    5 +++++
 arch/x86/kernel/smpboot.c    |    2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 4640ddd..638bf62 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -85,7 +85,8 @@ static inline void native_write_msr(unsigned int msr,
 	asm volatile("wrmsr" : : "c" (msr), "a"(low), "d" (high) : "memory");
 }
 
-static inline int native_write_msr_safe(unsigned int msr,
+/* Can be uninlined because referenced by paravirt */
+notrace static inline int native_write_msr_safe(unsigned int msr,
 					unsigned low, unsigned high)
 {
 	int err;
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index fc99173..c381330 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -2,6 +2,11 @@
 # Makefile for x86-compatible CPU details, features and quirks
 #
 
+# Don't trace early stages of a secondary CPU boot
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_common.o = -pg
+endif
+
 obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
 obj-y			+= proc.o capflags.o powerflags.o common.o
 obj-y			+= vmware.o hypervisor.o
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b1d571b..31869bf 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -282,7 +282,7 @@ static int __cpuinitdata unsafe_smp;
 /*
  * Activate a secondary processor.
  */
-static void __cpuinit start_secondary(void *unused)
+notrace static void __cpuinit start_secondary(void *unused)
 {
 	/*
 	 * Don't put *anything* before cpu_init(), SMP booting is too
-- 
1.6.0.4

Re: [PATCH v2] tracing/ftrace: don't trace on early stage of a secondary cpu boot

[Steven Rostedt]

Cool, so the CFLAGS_REMOVE solution worked.

I'm the last one to criticize someones language, especially when it is not 
their native tongue. The closest language outside of English that I know 
is German, and I would do horrible if I had to write this in German. Heck, 
my English sucks too ;-)

But since this is going to be a public record of what you changed, it 
should be a bit more clear.

On Wed, 24 Dec 2008, Frederic Weisbecker wrote:

> Impact: fix a crash/hard-reboot while enabling cpu on runtime
> 
> On some archs, the boot of a secondary cpu can have an early fragile state.
> On x86-64, the pda is not initialized on the first stage of a cpu boot but

s/on the first/in the first/

> it is needed to get the cpu number and the current task pointer. These datas

s/These datas are/This data is/

> are needed during tracing. As they were dereferenced at this stage, we got a
> crash while turning on a cpu on runtime while tracing.

"we got a crash while tracing a cpu being enabled at runtime"

> 
> Some other archs like ia64 can have such kind of issue too.
> 
> Changes on v2:
> 
> We drop the previous solution of a per-arch called function to guess the current state

s/drop/dropped/

> of a cpu. That could make slow the tracing.

"That could slow down the tracing."

> This patch just drop the -pg flag on arch/x86/kernel/cpu/common.c where

"This patch removes the -pg flag..."

> live the low level cpu boot functions, and on start_secondary() and a helper

"where the low level cpu boot functions exist"

s/, and on/, on/

> function used at this stage.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>

Again, I'm still impressed by your ability to communicate in a language 
other than your own. And perhaps my corrections are incorrect too ;-)

Acked-by: Steven Rostedt <srostedt@redhat.com>

-- Steve

Re: [PATCH v2] tracing/ftrace: don't trace on early stage of a secondary cpu boot

[Frederic Weisbecker]

Steven Rostedt wrote:
> Cool, so the CFLAGS_REMOVE solution worked.
> 
> I'm the last one to criticize someones language, especially when it is not 
> their native tongue. The closest language outside of English that I know 
> is German, and I would do horrible if I had to write this in German.


Me too. I can't imagine this. I studied more the german than the english language,
but I wouldn't dare send only one "Impact" line in german. It would be worst than my
english.


> 
> But since this is going to be a public record of what you changed, it 
> should be a bit more clear.

Of course. And thanks for these comments!


> On Wed, 24 Dec 2008, Frederic Weisbecker wrote:
> 
>> Impact: fix a crash/hard-reboot while enabling cpu on runtime
>>
>> On some archs, the boot of a secondary cpu can have an early fragile state.
>> On x86-64, the pda is not initialized on the first stage of a cpu boot but
> 
> s/on the first/in the first/
> 
>> it is needed to get the cpu number and the current task pointer. These datas
> 
> s/These datas are/This data is/
> 
>> are needed during tracing. As they were dereferenced at this stage, we got a
>> crash while turning on a cpu on runtime while tracing.
> 
> "we got a crash while tracing a cpu being enabled at runtime"
> 
>> Some other archs like ia64 can have such kind of issue too.
>>
>> Changes on v2:
>>
>> We drop the previous solution of a per-arch called function to guess the current state
> 
> s/drop/dropped/
> 
>> of a cpu. That could make slow the tracing.
> 
> "That could slow down the tracing."
> 
>> This patch just drop the -pg flag on arch/x86/kernel/cpu/common.c where
> 
> "This patch removes the -pg flag..."
> 
>> live the low level cpu boot functions, and on start_secondary() and a helper
> 
> "where the low level cpu boot functions exist"
> 
> s/, and on/, on/
> 
>> function used at this stage.
>>
>> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> Again, I'm still impressed by your ability to communicate in a language 
> other than your own. And perhaps my corrections are incorrect too ;-)
> 
> Acked-by: Steven Rostedt <srostedt@redhat.com>
> 
> -- Steve

Thanks. Note that french people like me are silently cheating with the english language.
We have a lot of similar words and we can survive while mumbling english, even with only few
basis, but don't tell anyone, it's a secret ;-)
 
I applied your comments, see the patch below.
Thanks again.
--

From: Frederic Weisbecker <fweisbec@gmail.com>
Subject: [PATCH v3] tracing/ftrace: don't trace on early stage of secondary cpu boot

Impact: fix a crash/hard-reboot while enabling cpu on runtime

On some archs, the boot of a secondary cpu can have an early fragile state.
On x86-64, the pda is not initialized on the first stage of a cpu boot but
it is needed to get the cpu number and the current task pointer. This data
is needed during tracing. As they were dereferenced at this stage, we got a
crash while tracing a cpu being enabled at runtime.

Some other archs like ia64 can have such kind of issue too.

Changes on v2:

We dropped the previous solution of a per-arch called function to guess the current state
of a cpu. That could slow down the tracing.
This patch removes the -pg flag on arch/x86/kernel/cpu/common.c where
the low level cpu boot functions exist, on start_secondary() and a helper
function used at this stage.

Changes on v3:

Fix my evil english in the v2 (Thanks Steven).

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Acked-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/x86/include/asm/msr.h   |    3 ++-
 arch/x86/kernel/cpu/Makefile |    5 +++++
 arch/x86/kernel/smpboot.c    |    2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 4640ddd..638bf62 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -85,7 +85,8 @@ static inline void native_write_msr(unsigned int msr,
 	asm volatile("wrmsr" : : "c" (msr), "a"(low), "d" (high) : "memory");
 }
 
-static inline int native_write_msr_safe(unsigned int msr,
+/* Can be uninlined because referenced by paravirt */
+notrace static inline int native_write_msr_safe(unsigned int msr,
 					unsigned low, unsigned high)
 {
 	int err;
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index fc99173..c381330 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -2,6 +2,11 @@
 # Makefile for x86-compatible CPU details, features and quirks
 #
 
+# Don't trace early stages of a secondary CPU boot
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_common.o = -pg
+endif
+
 obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
 obj-y			+= proc.o capflags.o powerflags.o common.o
 obj-y			+= vmware.o hypervisor.o
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b1d571b..31869bf 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -282,7 +282,7 @@ static int __cpuinitdata unsafe_smp;
 /*
  * Activate a secondary processor.
  */
-static void __cpuinit start_secondary(void *unused)
+notrace static void __cpuinit start_secondary(void *unused)
 {
 	/*
 	 * Don't put *anything* before cpu_init(), SMP booting is too
-- 
1.6.0.4

Re: [PATCH v2] tracing/ftrace: don't trace on early stage of a secondary cpu boot

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Thanks. Note that french people like me are silently cheating with the 
> english language. We have a lot of similar words and we can survive 
> while mumbling english, even with only few basis, but don't tell anyone, 
> it's a secret ;-)

heh.

I guess it helps that more than half a millenium ago the fine folks on 
those islands tried to learn French real hard [by virtue of being 
conquered by the French] - and thus a healthy subset of the English 
language (especially the more newfangled words) has thus become ... erm 
... French? ;-)

Doh, i misspoke. What happened _really_ is that those friendly folks let 
the French in as guests, and during that time of mutual understanding the 
French learned and adopted half of the English language - greatly easing 
communication today.

( And apparently there was some mingling with germanic tribes as well,
  a few thousand years before that. Back then those germanic savages were
  taught bits of proper English as well. )

( And then those folks forked their language into a "British" and "US" 
  versions. It is forwards compatible: if you speak British English then
  by definition you can speak US English, but never the other way around.
  US English was then taught to indian tribes - who, tens of thousands of
  years ago, happened to have spoken the same language that the ancestors
  of the germanic tribes spoke. [Or perhaps they spoke ancient Japanese - 
  memories are a bit fuzzy, records incomplete, and the issue is not fully 
  settled yet.] Anyway, it came around in a happy global circle and they 
  all speak English now! ;)

  [ Except the celts, who insist that they were around even sooner than 
    that - starting in the happy times when you could walk on feet from 
    Scotland to Sweden with only a battle axe in your hand, when you 
    could slide stones weighing tons down an ice glacier and ship them to 
    flatland to build funny stone circles - and from whom everyone else
    on this continent learned all the things worth learning. ]

> I applied your comments, see the patch below.
>
> From: Frederic Weisbecker <fweisbec@gmail.com>
> Subject: [PATCH v3] tracing/ftrace: don't trace on early stage of secondary cpu boot

applied to tip/tracing/ftrace, thanks Frederic!

	Ingo