linux-kernel - Re: [PATCH 2/3] tracing: block-able ring

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090901230508.GC6108@nowhere>
Date:	Wed, 2 Sep 2009 01:05:10 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Steven Rostedt <rostedt@...dmis.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] tracing: block-able ring_buffer consumer

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/