Re: [PATCH 2/3] tracing: block-able ring_buffer consumer

[Frederic Weisbecker <fweisbec@gmail.com>]

On Tue, Sep 01, 2009 at 08:53:03PM +0800, Lai Jiangshan wrote:
> >> @@ -1178,6 +1179,7 @@ void update_process_times(int user_tick)
> >>  	printk_tick();
> >>  	scheduler_tick();
> >>  	run_posix_cpu_timers(p);
> >> +	tracing_notify();
> > 
> > 
> > 
> > Hmm, that looks really not a good idea. The tracing shouldn't ever impact
> > the system when it is inactive.
> > Especially in such a fast path like the timer interrupt.
> 
> It do nothing at most time.
> It just calls tracing_notify() and then returns, but...
> it still has several mb()s, I can remove this mb()s in next patch.



The timer interrupt is one of the real fast path of the kernel.
Adding an extra call there plus some condition checks even when
the tracing is off is not a nice thing.

I bet we could even measure the overhead of this, especially
in a 1000 Hz kernel.

Also do we have a low latency requirement that justifies this check
on every tick? I don't think so. It's just about having tracing
results.

 
> We cannot call wake_up() and the tracing write side, so we delay
> the wake_up() until next timer interrupt.


Indeed we can't because of a random cpu rq lock that we may be
already holding.

May be try something similar to what we are doing for sensible tracers.

Normal tracers use default_wait_pipe() which uses a common wake up.
The others that may wake up while already holding a rq lock use
a polling wait:

void poll_wait_pipe(struct trace_iterator *iter)
{
	set_current_state(TASK_INTERRUPTIBLE);
	/* sleep for 100 msecs, and try again. */
	schedule_timeout(HZ / 10);
}

On the worst case the reader will wait for 1/100 secs
(the comment is wrong).

You can probably use the same thing for ring buffer splice
and poll waiters.

	Frederic.