In some rare cases, nmis are generated immediately after the nmi
handler of the cpu was started. This causes the counter not to be
enabled. Before enabling the nmi handlers we need to set variable
ctr_running first and make sure its value is written to memory.
Also, the patch makes all existing barriers a memory barrier instead
of a compiler barrier only.
Reported-by: Suravee Suthikulpanit <[email protected]>
Cc: <[email protected]> # .35+
Signed-off-by: Robert Richter <[email protected]>
---
arch/x86/oprofile/nmi_int.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index cf97500..68894fd 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -112,8 +112,10 @@ static void nmi_cpu_start(void *dummy)
static int nmi_start(void)
{
get_online_cpus();
- on_each_cpu(nmi_cpu_start, NULL, 1);
ctr_running = 1;
+ /* make ctr_running visible to the nmi handler: */
+ smp_mb();
+ on_each_cpu(nmi_cpu_start, NULL, 1);
put_online_cpus();
return 0;
}
@@ -504,15 +506,18 @@ static int nmi_setup(void)
nmi_enabled = 0;
ctr_running = 0;
- barrier();
+ /* make variables visible to the nmi handler: */
+ smp_mb();
err = register_die_notifier(&profile_exceptions_nb);
if (err)
goto fail;
get_online_cpus();
register_cpu_notifier(&oprofile_cpu_nb);
- on_each_cpu(nmi_cpu_setup, NULL, 1);
nmi_enabled = 1;
+ /* make nmi_enabled visible to the nmi handler: */
+ smp_mb();
+ on_each_cpu(nmi_cpu_setup, NULL, 1);
put_online_cpus();
return 0;
@@ -531,7 +536,8 @@ static void nmi_shutdown(void)
nmi_enabled = 0;
ctr_running = 0;
put_online_cpus();
- barrier();
+ /* make variables visible to the nmi handler: */
+ smp_mb();
unregister_die_notifier(&profile_exceptions_nb);
msrs = &get_cpu_var(cpu_msrs);
model->shutdown(msrs);
--
1.7.5.rc3
* Robert Richter <[email protected]> wrote:
> In some rare cases, nmis are generated immediately after the nmi
> handler of the cpu was started. This causes the counter not to be
> enabled. Before enabling the nmi handlers we need to set variable
> ctr_running first and make sure its value is written to memory.
>
> Also, the patch makes all existing barriers a memory barrier
> instead of a compiler barrier only.
Could we *PLEASE* just do the obvious and turn oprofile into a perf
user on x86 too, like ARM already does? Two years down the line and
there's zero progress in that area. We want to obsolete the Oprofile
PMU bits on x86, the infrastructure duplication causes non-trivial
problems like this one.
Thanks,
Ingo
On 09.06.11 03:18:54, Ingo Molnar wrote:
>
> * Robert Richter <[email protected]> wrote:
>
> > In some rare cases, nmis are generated immediately after the nmi
> > handler of the cpu was started. This causes the counter not to be
> > enabled. Before enabling the nmi handlers we need to set variable
> > ctr_running first and make sure its value is written to memory.
> >
> > Also, the patch makes all existing barriers a memory barrier
> > instead of a compiler barrier only.
>
> Could we *PLEASE* just do the obvious and turn oprofile into a perf
> user on x86 too, like ARM already does? Two years down the line and
> there's zero progress in that area. We want to obsolete the Oprofile
> PMU bits on x86, the infrastructure duplication causes non-trivial
> problems like this one.
Ingo,
I would rather prefer to spend the effort to add full pmu support to
perf. AMD features missing are still IBS and nb performance counters
for family 15h. The oprofile kernel driver would become obsolete at
all with a new daemon using the perf syscall interface. So why
spending more time with porting this legacy driver to perf in the
kernel? It will be also hard to implement identical behaviour which
may raise concerns about backward compatibility of the kernel driver.
I admit, we have had a lot of bug fixes for oprofile in the past, but
most of them are part of maintaining existing code which we have to do
for distros anyway. There is almost no or only limited feature
development for oprofile now.
Hope this makes sense.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
* Robert Richter <[email protected]> wrote:
> On 09.06.11 03:18:54, Ingo Molnar wrote:
> >
> > * Robert Richter <[email protected]> wrote:
> >
> > > In some rare cases, nmis are generated immediately after the
> > > nmi handler of the cpu was started. This causes the counter not
> > > to be enabled. Before enabling the nmi handlers we need to set
> > > variable ctr_running first and make sure its value is written
> > > to memory.
> > >
> > > Also, the patch makes all existing barriers a memory barrier
> > > instead of a compiler barrier only.
> >
> > Could we *PLEASE* just do the obvious and turn oprofile into a
> > perf user on x86 too, like ARM already does? Two years down the
> > line and there's zero progress in that area. We want to obsolete
> > the Oprofile PMU bits on x86, the infrastructure duplication
> > causes non-trivial problems like this one.
>
> Ingo,
>
> I would rather prefer to spend the effort to add full pmu support
> to perf. AMD features missing are still IBS and nb performance
> counters for family 15h. The oprofile kernel driver would become
> obsolete at all with a new daemon using the perf syscall interface.
Agreed - i just don't see *any* progress on the IBS front: more time
seems to be spent on fixing and sometimes even extending oprofile
than on any IBS work.
Thanks,
Ingo