* Entry conditions for perf_event_do_pending? @ 2010-01-13 4:14 Paul Mackerras 2010-01-13 9:27 ` Peter Zijlstra 0 siblings, 1 reply; 3+ messages in thread From: Paul Mackerras @ 2010-01-13 4:14 UTC (permalink / raw) To: Ingo Molnar, Peter Zijlstra Cc: linux-kernel, Milton Miller, Benjamin Herrenschmidt We're seeing some perf-related crashes on powerpc related to having irqs in an inconsistent state (soft-enable vs. hard-enable vs. trace-irqs state) when entering perf_event_do_pending(). We're fixing that, but along the way we have struck a question about what conditions are required on entry to perf_event_do_pending. Its use of __get_cpu_var implies that it at least needs to be called with either interrupts or preemption disabled. Does it in fact need to be called with irqs off? Do we need to call irq_enter()/irq_exit() around it? Are there any other requirements that people can think of? Paul. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Entry conditions for perf_event_do_pending? 2010-01-13 4:14 Entry conditions for perf_event_do_pending? Paul Mackerras @ 2010-01-13 9:27 ` Peter Zijlstra 2010-01-21 13:54 ` [tip:perf/urgent] perf: Fix perf_event_do_pending() fallback callsite tip-bot for Peter Zijlstra 0 siblings, 1 reply; 3+ messages in thread From: Peter Zijlstra @ 2010-01-13 9:27 UTC (permalink / raw) To: Paul Mackerras Cc: Ingo Molnar, linux-kernel, Milton Miller, Benjamin Herrenschmidt On Wed, 2010-01-13 at 15:14 +1100, Paul Mackerras wrote: > We're seeing some perf-related crashes on powerpc related to having > irqs in an inconsistent state (soft-enable vs. hard-enable > vs. trace-irqs state) when entering perf_event_do_pending(). > We're fixing that, but along the way we have struck a question about > what conditions are required on entry to perf_event_do_pending. > > Its use of __get_cpu_var implies that it at least needs to be called > with either interrupts or preemption disabled. Does it in fact need > to be called with irqs off? Do we need to call irq_enter()/irq_exit() > around it? Are there any other requirements that people can think of? Right, so when I wrote it all it required was preempt disabled, but then I added all that disable stuff (perf_pending_event(): event->pending_disable) and that requires IRQs disabled because it calls __perf_event_disable() which takes ctx->lock, which is supposed to be an IRQ safe lock. On x86 we always run it from a self-ipi, which is I guess why the generic timer softirq callback never triggered for me, because that looks broken. So in short, I think perf_event_do_pending() requires full IRQ context, if that includes calling irq_enter()/irq_exit() then yes. Something like the below ought to do I guess.. --- kernel/timer.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/kernel/timer.c b/kernel/timer.c index 15533b7..c61a794 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1198,6 +1198,7 @@ void update_process_times(int user_tick) run_local_timers(); rcu_check_callbacks(cpu, user_tick); printk_tick(); + perf_event_do_pending(); scheduler_tick(); run_posix_cpu_timers(p); } @@ -1209,8 +1210,6 @@ static void run_timer_softirq(struct softirq_action *h) { struct tvec_base *base = __get_cpu_var(tvec_bases); - perf_event_do_pending(); - hrtimer_run_pending(); if (time_after_eq(jiffies, base->timer_jiffies)) ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [tip:perf/urgent] perf: Fix perf_event_do_pending() fallback callsite 2010-01-13 9:27 ` Peter Zijlstra @ 2010-01-21 13:54 ` tip-bot for Peter Zijlstra 0 siblings, 0 replies; 3+ messages in thread From: tip-bot for Peter Zijlstra @ 2010-01-21 13:54 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, paulus, hpa, mingo, a.p.zijlstra, tglx, mingo Commit-ID: fe432200abb0d64f409895168d9ad8fbb9d8e6c6 Gitweb: http://git.kernel.org/tip/fe432200abb0d64f409895168d9ad8fbb9d8e6c6 Author: Peter Zijlstra <a.p.zijlstra@chello.nl> AuthorDate: Mon, 18 Jan 2010 09:08:26 +0100 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Thu, 21 Jan 2010 13:40:39 +0100 perf: Fix perf_event_do_pending() fallback callsite Paul questioned the context in which we should call perf_event_do_pending(). After looking at that I found that it should be called from IRQ context these days, however the fallback call-site is placed in softirq context. Ammend this by placing the callback in the IRQ timer path. Reported-by: Paul Mackerras <paulus@samba.org> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <1263374859.4244.192.camel@laptop> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- kernel/timer.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/kernel/timer.c b/kernel/timer.c index 15533b7..c61a794 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1198,6 +1198,7 @@ void update_process_times(int user_tick) run_local_timers(); rcu_check_callbacks(cpu, user_tick); printk_tick(); + perf_event_do_pending(); scheduler_tick(); run_posix_cpu_timers(p); } @@ -1209,8 +1210,6 @@ static void run_timer_softirq(struct softirq_action *h) { struct tvec_base *base = __get_cpu_var(tvec_bases); - perf_event_do_pending(); - hrtimer_run_pending(); if (time_after_eq(jiffies, base->timer_jiffies)) ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-01-21 13:55 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-13 4:14 Entry conditions for perf_event_do_pending? Paul Mackerras 2010-01-13 9:27 ` Peter Zijlstra 2010-01-21 13:54 ` [tip:perf/urgent] perf: Fix perf_event_do_pending() fallback callsite tip-bot for Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox