Re: [patch 00/21] mutex subsystem, -V14

From: Ingo Molnar <mingo@elte.hu>
To: Joel Schopp <jschopp@austin.ibm.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@osdl.org>, Andrew Morton <akpm@osdl.org>,
	Arjan van de Ven <arjan@infradead.org>,
	Nicolas Pitre <nico@cam.org>,
	Jes Sorensen <jes@trained-monkey.org>,
	Al Viro <viro@ftp.linux.org.uk>, Oleg Nesterov <oleg@tv-sign.ru>,
	David Howells <dhowells@redhat.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Christoph Hellwig <hch@infradead.org>, Andi Kleen <ak@suse.de>,
	Russell King <rmk+lkml@arm.linux.org.uk>,
	Anton Blanchard <anton@samba.org>
Subject: Re: [patch 00/21] mutex subsystem, -V14
Date: Thu, 5 Jan 2006 15:35:02 +0100	[thread overview]
Message-ID: <20060105143502.GA16816@elte.hu> (raw)
In-Reply-To: <43BC5E15.207@austin.ibm.com>

* Joel Schopp <jschopp@austin.ibm.com> wrote:

> Took a glance at this on ppc64.  Would it be useful if I contributed 
> an arch specific version like arm has?  We'll either need an arch 
> specific version or have the generic changed.

feel free to implement an assembly mutex fastpath, and it would 
certainly be welcome and useful - but i think you are wrong about SMP 
synchronization:

> Anyway, here is some disassembly of some of the code generated with my 
> comments:
> 
> c00000000049bf9c <.mutex_lock>:
> c00000000049bf9c:       7c 00 06 ac     eieio
> c00000000049bfa0:       7d 20 18 28     lwarx   r9,r0,r3
> c00000000049bfa4:       31 29 ff ff     addic   r9,r9,-1

> The eieio is completly unnecessary, it got picked up from 
> atomic_dec_return (Anton, why is there an eieio at the start of 
> atomic_dec_return in the first place?).

a mutex is like a spinlock, it must prevent loads and stores within the 
critical section from 'leaking outside the critical section' [they must 
not be reordered to before the mutex_lock(), nor to after the 
mutex_unlock()] - hence the barriers added by atomic_dec_return() are 
very much needed.

	Ingo