mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolas Pitre <nico@cam.org>
To: Russell King <rmk+lkml@arm.linux.org.uk>
Cc: Linus Torvalds <torvalds@osdl.org>,
	David Howells <dhowells@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-arch@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>,
	mingo@redhat.com, Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 1/12]: MUTEX: Implement mutexes
Date: Mon, 19 Dec 2005 10:45:43 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0512191009170.26663@localhost.localdomain> (raw)
In-Reply-To: <20051219092743.GA9609@flint.arm.linux.org.uk>

On Mon, 19 Dec 2005, Russell King wrote:

> On Sun, Dec 18, 2005 at 08:48:15PM -0500, Nicolas Pitre wrote:
> > On Sun, 18 Dec 2005, Linus Torvalds wrote:
> > > I agree, if arm interrupt disables are fast. For example, on x86 (where 
> > > this isn't needed, because you can have an "interrupt-safe" decrement by 
> > > just having it as a single instruction, even if it isn't SMP-safe), 
> > > disabling and re-enabling interrupts is just one instruction each, but the 
> > > combination is usually something like 50+ cycles. So if this was an issue 
> > > on x86, we'd definitely care.
> > > 
> > > But if you don't think it's a big issue on ARM, it just doesn't matter.
> > 
> > Let's see.  The core of the uncontended down() on ARM looks like this:
> > 
> > 	mrs     r0, cpsr
> > 	orr     r1, r0, #128
> > 	msr     cpsr_c, r1
> > 	ldr     r1, [%0]
> > 	subs     r1, r1, #1
> > 	str     r1, [%0]
> > 	msr     cpsr_c, r0
> > 	blt	__contention
> > 
> > On a 624MHz ARMv5 processor I can execute this sequence approximately 
> > 266100 times in 10 ms, which means approx 23 cycles.  The uncontended 
> > up() is the same except the sub is replaced by an add.
> > 
> > Removing the interrupt masking/unmasking reduces the above sequence to 4 
> > instructions using 6 cycles.
> 
> I think you're comparing applies with oranges here - you measured the
> above function by executing it, and the reduced version by some other
> method (you appear to be absolutely certain that it's 6 cycles, but
> the previous was approximate).

They were _all_ measured execution times (I learned not to trust the 
manual for those).  Here's the boring details in case you didn't trust 
me:

The above 6 cycles was deduced from executing the given sequence of 
instructions 1014100 times within 10 ms at 624 MHz.

> > Now if we consider simple mutexes, the core of it becomes this on ARM:
> > 
> > 	mov	r0, #1
> > 	swp	r1, r0, [%0]
> > 	cmp	r1, #0
> > 	bne	__contention
> > 
> > The above takes 8 cycles.  It uses 4 instructions, and it could even be 
> > reduced to 3 when gcc's cse optimization can find a register that 
> > already contains the value 1 (then using only 7 cycles).  It is 
> > interrupt safe.  It is preemption safe.  It is small.

And for completeness: 762700 times within 10 ms at 624 MHz.

> That's over-simplified, and is the easy bit.  Now work out how you handle
> the unlock operation.

I exposed that in a previous post in this thread already.

> You don't know whether the lock is contended or not in the unlock path,
> so you always have to do the "wake up" thing.  (You can't rely on the
> value of the lock since another thread may well be between this swp
> instruction and entering the __contention function.  Hence you can't
> use the value of the lock to determine whether there's anyone sleeping
> on it.)

No.  For simplicity in this mail I only provided the inline portion of 
the mutex.  Let me provide again the __contention part which should be 
out of line:

__contention:
	mov	r0, #2			@ 2 = contended lock flag
	swp	r1, r0, [%0]		@ upgrade the lock, get current value
	cmp	r1, #0			@ has it just been unlocked?
	moveq	pc, lr			@ if so we now own it.
	stmfd	sp!, {r2, r3, ip, lr}	@ save whatever regs (needs %0 too)
	bl	__sleep
	ldmfd	sp!, {r2, r3, ip, lr}
	mov	r0, #1			@ try to lock again
	swp	r1, r0, [%0]		@ and get current state
	cmp	r1, #0			@ if it was locked again
	bne	__contention		@ then we loop again
	mov	pc, lr			@ otherwise we own it at last

Note that it needs task state and wait queue handling added which means 
that part should probably have a C helper like the current semaphore 
implementation.  But you should have the basic idea now.

> Therefore, I suspect that while the lock may be faster, the unlock
> won't be.

Not at all.  The inlined unlock goes like this:

	mov	r0, #0			@ 0 = unlock flag
	swp	r1, r0, [%0]		@ unlock and get current state
	cmp	r1, #2			@ was it contended?
	bleq	__wake_up		@ if so wake up one thread

Again 4 instructions (possibly 3 if cse is possible) and therefore with 
the same 7-8 cycle cost.

The only issue has to do with the fact that the lock might have been 
unlocked right before we go to upgrade it to contended.  We already 
catch that case since by locking the mutex with the contended flag we 
effectively own it at that point.  But then a superfluous wake-up will 
be done even if there is no contention anymore.  But as you see this 
kind of false contention is harmless and should be extremely rare (the 
window for it is really small).


Nicolas

  parent reply	other threads:[~2005-12-19 15:45 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-16 23:13 [PATCH 0/12]: MUTEX: Introduce mutex implementation David Howells
2005-12-16 23:13 ` [PATCH 3/12]: MUTEX: Rename DECLARE_MUTEX for arch/ dir David Howells
2005-12-16 23:13 ` [PATCH 2/12]: MUTEX: Provide SWAP-based mutex for FRV David Howells
2005-12-16 23:13 ` [PATCH 1/12]: MUTEX: Implement mutexes David Howells
2005-12-17  3:58   ` Steven Rostedt
2005-12-17  7:35     ` Linus Torvalds
2005-12-17  7:55     ` Nick Piggin
2005-12-17 12:36       ` Steven Rostedt
2005-12-17 19:21     ` David Howells
2005-12-17 20:11       ` Linus Torvalds
2005-12-17 21:44         ` Russell King
2005-12-18  1:29         ` Nicolas Pitre
2005-12-18  2:34           ` Linus Torvalds
2005-12-18  4:07             ` Nicolas Pitre
2005-12-18  4:18               ` Steven Rostedt
2005-12-18  6:30               ` Linus Torvalds
2005-12-18  9:26                 ` Russell King
2005-12-18 18:42                   ` Linus Torvalds
2005-12-18 19:41                     ` James Bottomley
2005-12-18 19:54                       ` Linus Torvalds
2005-12-19  1:48                     ` Nicolas Pitre
2005-12-19  9:27                       ` Russell King
2005-12-19 13:54                         ` Matthew Wilcox
2005-12-19 15:49                           ` Nicolas Pitre
2005-12-19 15:45                         ` Nicolas Pitre [this message]
2005-12-18 17:29                 ` Nicolas Pitre
2005-12-18 13:38           ` Alan Cox
2005-12-18 17:21             ` Nicolas Pitre
2005-12-16 23:13 ` [PATCH 11/12]: MUTEX: Rename DECLARE_MUTEX for miscellaneous directories David Howells
2005-12-16 23:13 ` [PATCH 5/12]: MUTEX: Rename DECLARE_MUTEX for drivers/ dir, N thru Z David Howells
2005-12-16 23:13 ` [PATCH 7/12]: MUTEX: Rename DECLARE_MUTEX for include/asm-*/ dirs David Howells
2005-12-16 23:13 ` [PATCH 9/12]: MUTEX: Rename DECLARE_MUTEX for net/ dir David Howells
2005-12-16 23:13 ` [PATCH 10/12]: MUTEX: Rename DECLARE_MUTEX for sound/ dir David Howells
2005-12-16 23:13 ` [PATCH 6/12]: MUTEX: Rename DECLARE_MUTEX for fs/ dir David Howells
2005-12-16 23:13 ` [PATCH 4/12]: MUTEX: Rename DECLARE_MUTEX for drivers/ dir, A thru M David Howells
2005-12-16 23:13 ` [PATCH 8/12]: MUTEX: Rename DECLARE_MUTEX for kernel/ dir David Howells
2005-12-16 23:13 ` [PATCH 12/12]: MUTEX: Provide synchronisation primitive testing module David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0512191009170.26663@localhost.localdomain \
    --to=nico@cam.org \
    --cc=akpm@osdl.org \
    --cc=dhowells@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rmk+lkml@arm.linux.org.uk \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®