On Tue, May 27, 2014 at 04:09:48PM +0200, Stephane Eranian wrote: > On Tue, May 20, 2014 at 11:02 AM, Peter Zijlstra <[email protected]> wrote: > > Subject: perf: Fix mux_interval hrtimer wreckage > > From: Peter Zijlstra <[email protected]> > > Date: Tue May 20 10:09:32 CEST 2014 > > > > Thomas stumbled over the hrtimer_forward_now() in > > perf_event_mux_interval_ms_store() and noticed its broken-ness. > > > > You cannot just change the expiry time of an active timer, it will > > destroy the red-black tree order and cause havoc. > > > > Change it to (re)start the timer instead, (re)starting a timer will > > dequeue and enqueue a timer and therefore preserve rb-tree order. > > > > Since we cannot enqueue remotely, wrap the thing in > > cpu_function_call(), this however mandates that we restrict ourselves > > to online cpus. Also serialize the entire setting so we don't get > > multiple concurrent threads trying to update to different values. > > > > Also fix a problem in perf_mux_hrtimer_restart(), checking against > > hrtimer_active() can actually loose us the timer when timer->state == > > HRTIMER_STATE_CALLBACK and the callback has already decided NORESTART. > > > > Furthermore it doesn't make any sense to test > > hrtimer_callback_running() when we already tested hrtimer_active(), > > but with the above change, we explicitly must call it when > > callback_running. > > > > Lastly, rename a few functions: > > > > s/perf_cpu_hrtimer_/perf_mux_hrtimer_/ -- because I could not find > > the mux timer function > > > > s/\<hr\>/timer/ -- because that's the normal way of calling things. > > > > Fixes: 62b856397927 ("perf: Add sysfs entry to adjust multiplexing interval > > per PMU") > > Cc: Stephane Eranian <[email protected]> > > Reported-by: Thomas Gleixner <[email protected]> > > Signed-off-by: Peter Zijlstra <[email protected]> > > Link: http://lkml.kernel.org/n/[email protected] > > So, I tested this patch on tip.git and it panics my kernels as soon as > I multiplex > events. For instance running: > $ perf stat -e cycles,cycles,cycles,cycles,cycles,cycles dd > if=/dev/urandom of=/dev/null count=10000000 >
Yeah, I hadn't actually tested it, but I did find more hrtimer wreckage meanwhile and I've not yet figured out how to fix it so I put this on hold for a little while. I'll try and get the lot sorted soon though. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
