2008-01-23 16:09:55

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH 06/20 -v5] add notrace annotations for NMI routines

This annotates NMI functions with notrace. Some tracers may be able
to live with this, but some cannot. So we turn off NMI tracing.

One solution might be to make a notrace_nmi which would only turn
off NMI tracing if a trace utility needed it off.

Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>

---
arch/x86/kernel/nmi_32.c | 2 +-
arch/x86/kernel/nmi_64.c | 2 +-
arch/x86/kernel/traps_32.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-mcount.git/arch/x86/kernel/nmi_32.c
===================================================================
--- linux-mcount.git.orig/arch/x86/kernel/nmi_32.c 2008-01-23 10:26:11.000000000 -0500
+++ linux-mcount.git/arch/x86/kernel/nmi_32.c 2008-01-23 10:26:53.000000000 -0500
@@ -323,7 +323,7 @@ EXPORT_SYMBOL(touch_nmi_watchdog);

extern void die_nmi(struct pt_regs *, const char *msg);

-__kprobes int nmi_watchdog_tick(struct pt_regs * regs, unsigned reason)
+notrace __kprobes int nmi_watchdog_tick(struct pt_regs *regs, unsigned reason)
{

/*
Index: linux-mcount.git/arch/x86/kernel/nmi_64.c
===================================================================
--- linux-mcount.git.orig/arch/x86/kernel/nmi_64.c 2008-01-23 10:26:11.000000000 -0500
+++ linux-mcount.git/arch/x86/kernel/nmi_64.c 2008-01-23 10:26:53.000000000 -0500
@@ -314,7 +314,7 @@ void touch_nmi_watchdog(void)
touch_softlockup_watchdog();
}

-int __kprobes nmi_watchdog_tick(struct pt_regs * regs, unsigned reason)
+notrace __kprobes int nmi_watchdog_tick(struct pt_regs *regs, unsigned reason)
{
int sum;
int touched = 0;
Index: linux-mcount.git/arch/x86/kernel/traps_32.c
===================================================================
--- linux-mcount.git.orig/arch/x86/kernel/traps_32.c 2008-01-23 10:26:11.000000000 -0500
+++ linux-mcount.git/arch/x86/kernel/traps_32.c 2008-01-23 10:26:53.000000000 -0500
@@ -723,7 +723,7 @@ void __kprobes die_nmi(struct pt_regs *r
do_exit(SIGSEGV);
}

-static __kprobes void default_do_nmi(struct pt_regs * regs)
+static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
{
unsigned char reason = 0;

@@ -763,7 +763,7 @@ static __kprobes void default_do_nmi(str

static int ignore_nmis;

-fastcall __kprobes void do_nmi(struct pt_regs * regs, long error_code)
+notrace fastcall __kprobes void do_nmi(struct pt_regs *regs, long error_code)
{
int cpu;


--


2008-01-23 21:31:49

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 06/20 -v5] add notrace annotations for NMI routines

* Steven Rostedt ([email protected]) wrote:
> This annotates NMI functions with notrace. Some tracers may be able
> to live with this, but some cannot. So we turn off NMI tracing.
>
> One solution might be to make a notrace_nmi which would only turn
> off NMI tracing if a trace utility needed it off.
>
Is this still needed with the atomic clocksource read ?

> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> Signed-off-by: Steven Rostedt <[email protected]>
>
> ---
> arch/x86/kernel/nmi_32.c | 2 +-
> arch/x86/kernel/nmi_64.c | 2 +-
> arch/x86/kernel/traps_32.c | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> Index: linux-mcount.git/arch/x86/kernel/nmi_32.c
> ===================================================================
> --- linux-mcount.git.orig/arch/x86/kernel/nmi_32.c 2008-01-23 10:26:11.000000000 -0500
> +++ linux-mcount.git/arch/x86/kernel/nmi_32.c 2008-01-23 10:26:53.000000000 -0500
> @@ -323,7 +323,7 @@ EXPORT_SYMBOL(touch_nmi_watchdog);
>
> extern void die_nmi(struct pt_regs *, const char *msg);
>
> -__kprobes int nmi_watchdog_tick(struct pt_regs * regs, unsigned reason)
> +notrace __kprobes int nmi_watchdog_tick(struct pt_regs *regs, unsigned reason)
> {
>
> /*
> Index: linux-mcount.git/arch/x86/kernel/nmi_64.c
> ===================================================================
> --- linux-mcount.git.orig/arch/x86/kernel/nmi_64.c 2008-01-23 10:26:11.000000000 -0500
> +++ linux-mcount.git/arch/x86/kernel/nmi_64.c 2008-01-23 10:26:53.000000000 -0500
> @@ -314,7 +314,7 @@ void touch_nmi_watchdog(void)
> touch_softlockup_watchdog();
> }
>
> -int __kprobes nmi_watchdog_tick(struct pt_regs * regs, unsigned reason)
> +notrace __kprobes int nmi_watchdog_tick(struct pt_regs *regs, unsigned reason)
> {
> int sum;
> int touched = 0;
> Index: linux-mcount.git/arch/x86/kernel/traps_32.c
> ===================================================================
> --- linux-mcount.git.orig/arch/x86/kernel/traps_32.c 2008-01-23 10:26:11.000000000 -0500
> +++ linux-mcount.git/arch/x86/kernel/traps_32.c 2008-01-23 10:26:53.000000000 -0500
> @@ -723,7 +723,7 @@ void __kprobes die_nmi(struct pt_regs *r
> do_exit(SIGSEGV);
> }
>
> -static __kprobes void default_do_nmi(struct pt_regs * regs)
> +static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
> {
> unsigned char reason = 0;
>
> @@ -763,7 +763,7 @@ static __kprobes void default_do_nmi(str
>
> static int ignore_nmis;
>
> -fastcall __kprobes void do_nmi(struct pt_regs * regs, long error_code)
> +notrace fastcall __kprobes void do_nmi(struct pt_regs *regs, long error_code)
> {
> int cpu;
>
>
> --

--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-23 21:58:52

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 06/20 -v5] add notrace annotations for NMI routines



On Wed, 23 Jan 2008, Mathieu Desnoyers wrote:

> * Steven Rostedt ([email protected]) wrote:
> > This annotates NMI functions with notrace. Some tracers may be able
> > to live with this, but some cannot. So we turn off NMI tracing.
> >
> > One solution might be to make a notrace_nmi which would only turn
> > off NMI tracing if a trace utility needed it off.
> >
> Is this still needed with the atomic clocksource read ?

This never had to do with the clocksource. The tracer itself isn't atomic
against NMIs. hmm, it may actually be. It does a tracer disable for the
CPU by an atomic_inc and this would prevent the NMI from causing harm.

I'll test without this patch and see what happens when I turn on a high
rate of NMIS.

I still want to add a bit more notraces around for the simple
reason of cleaning up the output and prehaps speeding up the code a bit.

-- Steve

2008-01-26 05:26:15

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 06/20 -v5] add notrace annotations for NMI routines


On Wed, 23 Jan 2008, Mathieu Desnoyers wrote:

> * Steven Rostedt ([email protected]) wrote:
> > This annotates NMI functions with notrace. Some tracers may be able
> > to live with this, but some cannot. So we turn off NMI tracing.
> >
> > One solution might be to make a notrace_nmi which would only turn
> > off NMI tracing if a trace utility needed it off.
> >
> Is this still needed with the atomic clocksource read ?
>

Before you ask again, I've still included this in -v6, simply because I
didn't get a chance to test it without this patch. I'll try to remember to
do that on Monday.

-- Steve

2008-01-28 11:59:17

by Jan Kiszka

[permalink] [raw]
Subject: Re: [PATCH 06/20 -v5] add notrace annotations for NMI routines

Steven Rostedt wrote:
> On Wed, 23 Jan 2008, Mathieu Desnoyers wrote:
>
>> * Steven Rostedt ([email protected]) wrote:
>>> This annotates NMI functions with notrace. Some tracers may be able
>>> to live with this, but some cannot. So we turn off NMI tracing.
>>>
>>> One solution might be to make a notrace_nmi which would only turn
>>> off NMI tracing if a trace utility needed it off.
>>>
>> Is this still needed with the atomic clocksource read ?
>>
>
> Before you ask again, I've still included this in -v6, simply because I
> didn't get a chance to test it without this patch. I'll try to remember to
> do that on Monday.

Only in case you finally have to keep the annotations for the posted
tracer: Then please make it a Kconfig selectable restriction, because
there are use cases of the mcount hook where you are interested in NMI
invocations.

TiA,
Jan

--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

2008-01-28 12:16:18

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 06/20 -v5] add notrace annotations for NMI routines


On Mon, 28 Jan 2008, Jan Kiszka wrote:
> Steven Rostedt wrote:
> > On Wed, 23 Jan 2008, Mathieu Desnoyers wrote:
> >
> >> * Steven Rostedt ([email protected]) wrote:
> >>> This annotates NMI functions with notrace. Some tracers may be able
> >>> to live with this, but some cannot. So we turn off NMI tracing.
> >>>
> >>> One solution might be to make a notrace_nmi which would only turn
> >>> off NMI tracing if a trace utility needed it off.
> >>>
> >> Is this still needed with the atomic clocksource read ?
> >>
> >
> > Before you ask again, I've still included this in -v6, simply because I
> > didn't get a chance to test it without this patch. I'll try to remember to
> > do that on Monday.
>
> Only in case you finally have to keep the annotations for the posted
> tracer: Then please make it a Kconfig selectable restriction, because
> there are use cases of the mcount hook where you are interested in NMI
> invocations.

I've thought about adding a notrace_nmi which would do precisely that.
This week I'll take this patch out and turn on a bunch of nmi profiling
and see how it holds up.

Thanks,

-- Steve