lkml.org 
[lkml]   [2008]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] x86/oprofile: disable preemption in nmi_shutdown
Date
Hi Vegard,

Vegard Nossum <vegard.nossum@gmail.com> writes:

> Hi,
>
> Does this look correct? I didn't really play with preemption before, but
> as far as I can tell, this is the right thing to do.
>
> I don't really get why model->shutdown(msrs) is done only for one of the
> CPUs, but my patch assumes that this is correct. (If that had been done
> from inside nmi_shutdown() for each CPU, we wouldn't have had to get the
> cpu var, and not needed to disable preemption.)
>
> Please comment :-)
>
>
> Vegard
>
>
> From: Vegard Nossum <vegard.nossum@gmail.com>
> Date: Sat, 21 Jun 2008 23:44:19 +0200
> Subject: [PATCH] x86/oprofile: disable preemption in nmi_shutdown
>
> BUG: using smp_processor_id() in preemptible [00000000] code: oprofiled/27301
> caller is nmi_shutdown+0x11/0x60
> Pid: 27301, comm: oprofiled Not tainted 2.6.26-rc7 #25
> [<c028a90d>] debug_smp_processor_id+0xbd/0xc0
> [<c045fba1>] nmi_shutdown+0x11/0x60
> [<c045dd4a>] oprofile_shutdown+0x2a/0x60
>
> Note that we don't need this for the other functions, since they are all
> called with on_each_cpu() (which disables preemption for us anyway).
>
> Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
> ---
> arch/x86/oprofile/nmi_int.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
> index cc48d3f..4a177b4 100644
> --- a/arch/x86/oprofile/nmi_int.c
> +++ b/arch/x86/oprofile/nmi_int.c
> @@ -269,12 +269,16 @@ static void nmi_cpu_shutdown(void *dummy)
>
> static void nmi_shutdown(void)
> {
> - struct op_msrs *msrs = &__get_cpu_var(cpu_msrs);
> + struct op_msrs *msrs;
> +
> + preempt_disable();
> + msrs = &__get_cpu_var(cpu_msrs);
> nmi_enabled = 0;
> on_each_cpu(nmi_cpu_shutdown, NULL, 0, 1);
> unregister_die_notifier(&profile_exceptions_nb);
> model->shutdown(msrs);
> free_msrs();
> + preempt_enable();

Have a look at get_cpu_var() and put_cpu_var(), that is exactly the
pattern.

Hannes


\
 
 \ /
  Last update: 2008-06-22 09:35    [W:0.075 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site