Re: [PATCH] locking/mutexes: Revert "locking/mutexes: Add extra reschedule point"

[Peter Zijlstra <peterz@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Thu, Jul 31, 2014 at 03:44:11PM +0200, Ingo Molnar wrote:
> 
> > diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> > index ae712b25e492..3d726fdaa764 100644
> > --- a/kernel/locking/mutex.c
> > +++ b/kernel/locking/mutex.c
> > @@ -473,8 +473,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
> >  	 * reschedule now, before we try-lock the mutex. This avoids getting
> >  	 * scheduled out right after we obtained the mutex.
> >  	 */
> > -	if (need_resched())
> > +	if (need_resched()) {
> > +		if (WARN_ON_ONCE(current->state != TASK_RUNNING))
> > +			__set_current_state(TASK_RUNNING);
> > +
> >  		schedule_preempt_disabled();
> > +	}
> 
> Might make sense to add that debug check under mutex debugging or so, 
> with a sensible kernel message printed.

Something like so? I suppose we should do a similar one for rwsem,
semaphores and possibly wait_event*() too.

---
Subject: locking/mutex: Add debug check for task state

Calling blocking locks with current->state != TASK_RUNNING is a bug.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/mutex.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index ae712b25e492..d5daf8c38899 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -375,6 +375,17 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 	unsigned long flags;
 	int ret;
 
+#ifdef CONFIG_DEBUG_MUTEXES
+	/*
+	 * Blocking primitives (including this one) will set (and therefore
+	 * destroy) current->state, since we will exit with TASK_RUNNING
+	 * make sure we enter with it, otherwise we will destroy state.
+	 */
+	if (WARN_ONCE(current->state != TASK_RUNNING,
+			"do not call blocking locks when !TASK_RUNNING\n"))
+		__set_current_state(TASK_RUNNING);
+#endif
+
 	preempt_disable();
 	mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
 

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]