Messages in this thread | | | Date | Tue, 2 Jun 2009 09:56:37 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] perf_counter: Provide functions for locking and pinning the context for a task |
| |
* Paul Mackerras <paulus@samba.org> wrote:
> Ingo Molnar writes: > > > Yeah, indeed that box has a CPU hotplug testcase - sets cpu1 to > > offline then online. > > > > There should be no counters active anywhere during that. > > OK, I can't reproduce this on powerpc. I guess you have dynamic > per-cpu patches in there, and per-cpu areas are getting > reinitialized when cpus come up. That, combined with the fact > that the migration_notifier in kernel/sched.c puts itself at > priority 10, means that we're getting a call to > perf_counter_task_migration() for a newly-added CPU before > perf_cpu_notify() has been called for that CPU, and so we're > trying to use an uninitialized perf_cpu_context and we go boom.
Sounds very plausible.
> Could you try the same test with this patch? If this fixes it, > then that's what the problem is. It's up to you whether > increasing the priority on perf_cpu_nb is the right solution or > whether we should solve the problem some other way. > > Paul. > > diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c > --- a/kernel/perf_counter.c > +++ b/kernel/perf_counter.c > @@ -3902,8 +3902,12 @@ perf_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu) > return NOTIFY_OK; > } > > +/* > + * This has to have a higher priority than migration_notifier in sched.c. > + */ > static struct notifier_block __cpuinitdata perf_cpu_nb = { > .notifier_call = perf_cpu_notify, > + .priority = 20, > };
Makes sense. Mind doing a full patch with a changelog, and with a comment that explains what the priority rules are? Perhaps add a comment to the counterpart in sched.c too.
Ingo
| |