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

Peter Zijlstra
2014-07-31T13:56:31Z

[-- 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 --]