linux-kernel - Re: [PATCH 2/2] ring_buffer: implement new locking

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20081001060126.GB25703@elte.hu>
Date:	Wed, 1 Oct 2008 08:01:26 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Mathieu Desnoyers <compudj@...stal.dyndns.org>,
	Pekka Paalanen <pq@....fi>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Martin Bligh <mbligh@...gle.com>,
	Steven Rostedt <srostedt@...hat.com>
Subject: Re: [PATCH 2/2] ring_buffer: implement new locking


* Steven Rostedt <rostedt@...dmis.org> wrote:

> The old "lock always" scheme had issues with lockdep, and was not very 
> efficient anyways.
> 
> This patch does a new design to be partially lockless on writes. 
> Writes will add new entries to the per cpu pages by simply disabling 
> interrupts. When a write needs to go to another page than it will grab 
> the lock.
> 
> A new "read page" has been added so that the reader can pull out a 
> page from the ring buffer to read without worrying about the writer 
> writing over it. This allows us to not take the lock for all reads. 
> The lock is now only taken when a read needs to go to a new page.
> 
> This is far from lockless, and interrupts still need to be disabled, 
> but it is a step towards a more lockless solution, and it also solves 
> a lot of the issues that were noticed by the first conversion of 
> ftrace to the ring buffers.
> 
> Note: the ring_buffer_{un}lock API has been removed.
> 
> Signed-off-by: Steven Rostedt <srostedt@...hat.com>
> ---
>  include/linux/ring_buffer.h |    3 
>  kernel/trace/ring_buffer.c  |  298 +++++++++++++++++++++++++-------------------
>  kernel/trace/trace.c        |  113 +++++++++++-----
>  3 files changed, 247 insertions(+), 167 deletions(-)

applied to tip/tracing/ring-buffer, thanks Steve!

the simplification factor is significant as well - both in linecount and 
in locking complexity. A robust tracer almost never wants to impose 
heavy locking semantics - a good way to disable/enable it on a per CPU 
basis is more than enough to 'freeze' the trace and get it out to the 
user.

Perhaps, before we free trace buffers, we should do an RCU sync to make 
sure no access can linger? Right now we dont seem to free buffers 
anywhere.

	Ingo
--
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/