mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Hugh Dickins <hugh@veritas.com>
Cc: linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@osdl.org>
Subject: [patch] fix i386 mutex fastpath on FRAME_POINTER && !DEBUG_MUTEXES
Date: Tue, 10 Jan 2006 22:07:44 +0100	[thread overview]
Message-ID: <20060110210744.GA8850@elte.hu> (raw)
In-Reply-To: <Pine.LNX.4.61.0601101718570.2942@goblin.wat.veritas.com>


* Hugh Dickins <hugh@veritas.com> wrote:

> I find the 2.6.15-git6 i386 CONFIG_FRAME_POINTER=y doesn't work unless 
> CONFIG_DEBUG_MUTEXES=y, because of the __mutex_fastpath jumps to 
> fail_fn (giving two pushes of %ebp to one pop).  Whereas x86_64 
> __mutex_fastpaths use calls and work with FRAME_POINTER=y.  Whether 
> i386 deserves asm mods rather than Kconfigery to force one from other, 
> I've no strong instinct.

ah, good eyes! This explains some of the crashes i saw today. The patch 
below solves it. Build and boot tested on x86. Linus, please apply.

	Ingo

--
call the mutex slowpath more conservatively - e.g. FRAME_POINTERS can
change the calling convention, in which case a direct branch to the
slowpath becomes illegal. Bug found by Hugh Dickins.

Signed-off-by: Ingo Molnar <mingo@elte.hu>

----

 include/asm-i386/mutex.h |   16 ++++++++++++++--
 kernel/mutex.c           |    9 ---------
 2 files changed, 14 insertions(+), 11 deletions(-)

Index: linux/include/asm-i386/mutex.h
===================================================================
--- linux.orig/include/asm-i386/mutex.h
+++ linux/include/asm-i386/mutex.h
@@ -28,7 +28,13 @@ do {									\
 									\
 	__asm__ __volatile__(						\
 		LOCK	"   decl (%%eax)	\n"			\
-			"   js "#fail_fn"	\n"			\
+			"   js 2f		\n"			\
+			"1:			\n"			\
+									\
+		LOCK_SECTION_START("")					\
+			"2: call "#fail_fn"	\n"			\
+			"   jmp 1b		\n"			\
+		LOCK_SECTION_END					\
 									\
 		:"=a" (dummy)						\
 		: "a" (count)						\
@@ -78,7 +84,13 @@ do {									\
 									\
 	__asm__ __volatile__(						\
 		LOCK	"   incl (%%eax)	\n"			\
-			"   jle "#fail_fn"	\n"			\
+			"   jle 2f		\n"			\
+			"1:			\n"			\
+									\
+		LOCK_SECTION_START("")					\
+			"2: call "#fail_fn"	\n"			\
+			"   jmp 1b		\n"			\
+		LOCK_SECTION_END					\
 									\
 		:"=a" (dummy)						\
 		: "a" (count)						\
Index: linux/kernel/mutex.c
===================================================================
--- linux.orig/kernel/mutex.c
+++ linux/kernel/mutex.c
@@ -84,12 +84,6 @@ void fastcall __sched mutex_lock(struct 
 	/*
 	 * The locking fastpath is the 1->0 transition from
 	 * 'unlocked' into 'locked' state.
-	 *
-	 * NOTE: if asm/mutex.h is included, then some architectures
-	 * rely on mutex_lock() having _no other code_ here but this
-	 * fastpath. That allows the assembly fastpath to do
-	 * tail-merging optimizations. (If you want to put testcode
-	 * here, do it under #ifndef CONFIG_MUTEX_DEBUG.)
 	 */
 	__mutex_fastpath_lock(&lock->count, __mutex_lock_slowpath);
 }
@@ -115,8 +109,6 @@ void fastcall __sched mutex_unlock(struc
 	/*
 	 * The unlocking fastpath is the 0->1 transition from 'locked'
 	 * into 'unlocked' state:
-	 *
-	 * NOTE: no other code must be here - see mutex_lock() .
 	 */
 	__mutex_fastpath_unlock(&lock->count, __mutex_unlock_slowpath);
 }
@@ -261,7 +253,6 @@ __mutex_lock_interruptible_slowpath(atom
  */
 int fastcall __sched mutex_lock_interruptible(struct mutex *lock)
 {
-	/* NOTE: no other code must be here - see mutex_lock() */
 	return __mutex_fastpath_lock_retval
 			(&lock->count, __mutex_lock_interruptible_slowpath);
 }

  reply	other threads:[~2006-01-10 21:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-10 17:44 i386 FRAME_POINTER needs DEBUG_MUTEXES Hugh Dickins
2006-01-10 21:07 ` Ingo Molnar [this message]
2006-01-11  8:24 [patch] fix i386 mutex fastpath on FRAME_POINTER && !DEBUG_MUTEXES Chuck Ebbert
2006-01-11 10:13 ` Ingo Molnar
2006-01-12 17:53 Chuck Ebbert

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=20060110210744.GA8850@elte.hu \
    --to=mingo@elte.hu \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.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®