public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Andrew Morton <akpm@osdl.org>
Cc: ntl@pobox.com, anton@au1.ibm.com, linux-kernel@vger.kernel.org,
	michael@ellerman.id.au, linuxppc64-dev@ozlabs.org,
	serue@us.ibm.com, paulus@au1.ibm.com, torvalds@osdl.org
Subject: [patch] work around ppc64 bootup bug by making mutex-debugging save/restore irqs
Date: Wed, 18 Jan 2006 10:02:43 +0100	[thread overview]
Message-ID: <20060118090243.GA15165@elte.hu> (raw)
In-Reply-To: <20060118002459.3bc8f75a.akpm@osdl.org>


* Andrew Morton <akpm@osdl.org> wrote:

> > Does it hide bugs that dont normally trigger 
> > during bootups on real hardware, but which could trigger on e.g. UML or 
> > on Xen? I really think such ugly workarounds are not justified, if other 
> > arches can get their act together. Would you make such an exception for 
> > other arches too, like ARM?
> 
> Don't care really, as long as a) the problems don't hit -mm or 
> mainline and b) someone else fixes them.  Yes, it'd be nice to fix 
> these things, and we might even find real bugs.  Perhaps things are 
> better now, but I suspect it's a can of worms.

patch to mutex code below.

-- 

it seems ppc64 wants to lock mutexes in early bootup code, with 
interrupts disabled, and they expect interrupts to stay disabled, else 
they crash.

work this bug around by making mutex debugging variants save/restore irq 
flags.

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

----

 kernel/mutex-debug.c |   12 ++++++------
 kernel/mutex-debug.h |   25 +++++--------------------
 kernel/mutex.c       |   21 ++++++++++++---------
 kernel/mutex.h       |    6 ++++--
 4 files changed, 27 insertions(+), 37 deletions(-)

Index: linux/kernel/mutex-debug.c
===================================================================
--- linux.orig/kernel/mutex-debug.c
+++ linux/kernel/mutex-debug.c
@@ -153,13 +153,13 @@ next:
 			continue;
 		count++;
 		cursor = curr->next;
-		debug_spin_lock_restore(&debug_mutex_lock, flags);
+		debug_spin_unlock_restore(&debug_mutex_lock, flags);
 
 		printk("\n#%03d:            ", count);
 		printk_lock(lock, filter ? 0 : 1);
 		goto next;
 	}
-	debug_spin_lock_restore(&debug_mutex_lock, flags);
+	debug_spin_unlock_restore(&debug_mutex_lock, flags);
 	printk("\n");
 }
 
@@ -316,7 +316,7 @@ void mutex_debug_check_no_locks_held(str
 			continue;
 		list_del_init(curr);
 		DEBUG_OFF();
-		debug_spin_lock_restore(&debug_mutex_lock, flags);
+		debug_spin_unlock_restore(&debug_mutex_lock, flags);
 
 		printk("BUG: %s/%d, lock held at task exit time!\n",
 			task->comm, task->pid);
@@ -325,7 +325,7 @@ void mutex_debug_check_no_locks_held(str
 			printk("exiting task is not even the owner??\n");
 		return;
 	}
-	debug_spin_lock_restore(&debug_mutex_lock, flags);
+	debug_spin_unlock_restore(&debug_mutex_lock, flags);
 }
 
 /*
@@ -352,7 +352,7 @@ void mutex_debug_check_no_locks_freed(co
 			continue;
 		list_del_init(curr);
 		DEBUG_OFF();
-		debug_spin_lock_restore(&debug_mutex_lock, flags);
+		debug_spin_unlock_restore(&debug_mutex_lock, flags);
 
 		printk("BUG: %s/%d, active lock [%p(%p-%p)] freed!\n",
 			current->comm, current->pid, lock, from, to);
@@ -362,7 +362,7 @@ void mutex_debug_check_no_locks_freed(co
 			printk("freeing task is not even the owner??\n");
 		return;
 	}
-	debug_spin_lock_restore(&debug_mutex_lock, flags);
+	debug_spin_unlock_restore(&debug_mutex_lock, flags);
 }
 
 /*
Index: linux/kernel/mutex-debug.h
===================================================================
--- linux.orig/kernel/mutex-debug.h
+++ linux/kernel/mutex-debug.h
@@ -46,21 +46,6 @@ extern void mutex_remove_waiter(struct m
 extern void debug_mutex_unlock(struct mutex *lock);
 extern void debug_mutex_init(struct mutex *lock, const char *name);
 
-#define debug_spin_lock(lock)				\
-	do {						\
-		local_irq_disable();			\
-		if (debug_mutex_on)			\
-			spin_lock(lock);		\
-	} while (0)
-
-#define debug_spin_unlock(lock)				\
-	do {						\
-		if (debug_mutex_on)			\
-			spin_unlock(lock);		\
-		local_irq_enable();			\
-		preempt_check_resched();		\
-	} while (0)
-
 #define debug_spin_lock_save(lock, flags)		\
 	do {						\
 		local_irq_save(flags);			\
@@ -68,7 +53,7 @@ extern void debug_mutex_init(struct mute
 			spin_lock(lock);		\
 	} while (0)
 
-#define debug_spin_lock_restore(lock, flags)		\
+#define debug_spin_unlock_restore(lock, flags)		\
 	do {						\
 		if (debug_mutex_on)			\
 			spin_unlock(lock);		\
@@ -76,20 +61,20 @@ extern void debug_mutex_init(struct mute
 		preempt_check_resched();		\
 	} while (0)
 
-#define spin_lock_mutex(lock)				\
+#define spin_lock_mutex(lock, flags)			\
 	do {						\
 		struct mutex *l = container_of(lock, struct mutex, wait_lock); \
 							\
 		DEBUG_WARN_ON(in_interrupt());		\
-		debug_spin_lock(&debug_mutex_lock);	\
+		debug_spin_lock_save(&debug_mutex_lock, flags); \
 		spin_lock(lock);			\
 		DEBUG_WARN_ON(l->magic != l);		\
 	} while (0)
 
-#define spin_unlock_mutex(lock)				\
+#define spin_unlock_mutex(lock, flags)			\
 	do {						\
 		spin_unlock(lock);			\
-		debug_spin_unlock(&debug_mutex_lock);	\
+		debug_spin_unlock_restore(&debug_mutex_lock, flags);	\
 	} while (0)
 
 #define DEBUG_OFF()					\
Index: linux/kernel/mutex.c
===================================================================
--- linux.orig/kernel/mutex.c
+++ linux/kernel/mutex.c
@@ -125,10 +125,11 @@ __mutex_lock_common(struct mutex *lock, 
 	struct task_struct *task = current;
 	struct mutex_waiter waiter;
 	unsigned int old_val;
+	unsigned long flags;
 
 	debug_mutex_init_waiter(&waiter);
 
-	spin_lock_mutex(&lock->wait_lock);
+	spin_lock_mutex(&lock->wait_lock, flags);
 
 	debug_mutex_add_waiter(lock, &waiter, task->thread_info, ip);
 
@@ -157,7 +158,7 @@ __mutex_lock_common(struct mutex *lock, 
 		if (unlikely(state == TASK_INTERRUPTIBLE &&
 						signal_pending(task))) {
 			mutex_remove_waiter(lock, &waiter, task->thread_info);
-			spin_unlock_mutex(&lock->wait_lock);
+			spin_unlock_mutex(&lock->wait_lock, flags);
 
 			debug_mutex_free_waiter(&waiter);
 			return -EINTR;
@@ -165,9 +166,9 @@ __mutex_lock_common(struct mutex *lock, 
 		__set_task_state(task, state);
 
 		/* didnt get the lock, go to sleep: */
-		spin_unlock_mutex(&lock->wait_lock);
+		spin_unlock_mutex(&lock->wait_lock, flags);
 		schedule();
-		spin_lock_mutex(&lock->wait_lock);
+		spin_lock_mutex(&lock->wait_lock, flags);
 	}
 
 	/* got the lock - rejoice! */
@@ -178,7 +179,7 @@ __mutex_lock_common(struct mutex *lock, 
 	if (likely(list_empty(&lock->wait_list)))
 		atomic_set(&lock->count, 0);
 
-	spin_unlock_mutex(&lock->wait_lock);
+	spin_unlock_mutex(&lock->wait_lock, flags);
 
 	debug_mutex_free_waiter(&waiter);
 
@@ -203,10 +204,11 @@ static fastcall noinline void
 __mutex_unlock_slowpath(atomic_t *lock_count __IP_DECL__)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
+	unsigned long flags;
 
 	DEBUG_WARN_ON(lock->owner != current_thread_info());
 
-	spin_lock_mutex(&lock->wait_lock);
+	spin_lock_mutex(&lock->wait_lock, flags);
 
 	/*
 	 * some architectures leave the lock unlocked in the fastpath failure
@@ -231,7 +233,7 @@ __mutex_unlock_slowpath(atomic_t *lock_c
 
 	debug_mutex_clear_owner(lock);
 
-	spin_unlock_mutex(&lock->wait_lock);
+	spin_unlock_mutex(&lock->wait_lock, flags);
 }
 
 /*
@@ -276,9 +278,10 @@ __mutex_lock_interruptible_slowpath(atom
 static inline int __mutex_trylock_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
+	unsigned long flags;
 	int prev;
 
-	spin_lock_mutex(&lock->wait_lock);
+	spin_lock_mutex(&lock->wait_lock, flags);
 
 	prev = atomic_xchg(&lock->count, -1);
 	if (likely(prev == 1))
@@ -287,7 +290,7 @@ static inline int __mutex_trylock_slowpa
 	if (likely(list_empty(&lock->wait_list)))
 		atomic_set(&lock->count, 0);
 
-	spin_unlock_mutex(&lock->wait_lock);
+	spin_unlock_mutex(&lock->wait_lock, flags);
 
 	return prev == 1;
 }
Index: linux/kernel/mutex.h
===================================================================
--- linux.orig/kernel/mutex.h
+++ linux/kernel/mutex.h
@@ -9,8 +9,10 @@
  * !CONFIG_DEBUG_MUTEXES case. Most of them are NOPs:
  */
 
-#define spin_lock_mutex(lock)			spin_lock(lock)
-#define spin_unlock_mutex(lock)			spin_unlock(lock)
+#define spin_lock_mutex(lock, flags) \
+		do { spin_lock(lock); (void)(flags); } while (0)
+#define spin_unlock_mutex(lock, flags) \
+		do { spin_unlock(lock); (void)(flags); } while (0)
 #define mutex_remove_waiter(lock, waiter, ti) \
 		__list_del((waiter)->list.prev, (waiter)->list.next)
 

  reply	other threads:[~2006-01-18  9:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-16  6:35 2.6.15-mm4 failure on power5 Serge E. Hallyn
2006-01-16  7:05 ` Andrew Morton
2006-01-16 13:00   ` Michael Ellerman
2006-01-16 15:37     ` Serge E. Hallyn
2006-01-16 21:52       ` Dave C Boutcher
2006-01-17  1:09         ` Andrew Morton
2006-01-17  8:17           ` Ingo Molnar
2006-01-17  8:47             ` Andrew Morton
2006-01-17 16:52             ` Dave C Boutcher
2006-01-17 16:55               ` Dave C Boutcher
2006-01-18  6:40                 ` Nathan Lynch
2006-01-18  7:07                   ` Ingo Molnar
2006-01-18  7:53                     ` Nathan Lynch
2006-01-18  8:08                   ` Nathan Lynch
2006-01-17 12:22         ` Serge E. Hallyn
2006-01-17 13:32         ` Michael Ellerman
2006-01-17 14:00           ` Ingo Molnar
2006-01-18  0:19             ` Michael Ellerman
2006-01-18  3:32               ` Dave C Boutcher
2006-01-18  6:37                 ` Ingo Molnar
2006-01-18  6:53                   ` Andrew Morton
2006-01-18  7:04                     ` Ingo Molnar
2006-01-18  7:28                     ` Nathan Lynch
2006-01-18  7:37                       ` Andrew Morton
2006-01-18  8:08                         ` Ingo Molnar
2006-01-18  8:24                           ` Andrew Morton
2006-01-18  9:02                             ` Ingo Molnar [this message]
2006-01-18  9:18                             ` [patch] turn on might_sleep() in early bootup code too Ingo Molnar
2006-01-18 10:35                               ` Andrew Morton
2006-01-18 10:43                                 ` Ingo Molnar
2006-01-18 11:15                                   ` [patch] make bug messages more consistent Ingo Molnar
2006-01-19  4:39                                   ` [patch] turn on might_sleep() in early bootup code too Zwane Mwaikambo
2006-01-18 10:46                                 ` Nick Piggin
2006-01-18 11:07                                   ` Ingo Molnar
2006-01-18 12:53                                     ` [patch] add trylock_kernel() Ingo Molnar
2006-01-18  7:38                       ` 2.6.15-mm4 failure on power5 Arjan van de Ven

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=20060118090243.GA15165@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=anton@au1.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc64-dev@ozlabs.org \
    --cc=michael@ellerman.id.au \
    --cc=ntl@pobox.com \
    --cc=paulus@au1.ibm.com \
    --cc=serue@us.ibm.com \
    --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