On Wed, 2009-01-07 at 10:22 -0500, Steven Rostedt wrote:
> Peter, nice work!

Thanks!

> > +   }
> > +
> > +   if (!spin) {
> > +           schedstat_inc(this_rq(), mtx_sched);
> > +           __set_task_state(task, state);
> 
> I still do not know why you set state here instead of in the mutex code.
> Yes, you prevent changing the state if we do not schedule, but there's 
> nothing wrong with setting it before hand. We may even be able to cache 
> the owner and keep the locking of the wait_lock out of here. But then I 
> see that it may be used to protect the sched_stat counters.

I was about to say because we need task_rq(owner) and can only deref
owner while holding that lock, but I found a way around it by using
task_cpu() which is exported.

Compile tested only so far...

---
Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -329,8 +329,8 @@ extern signed long schedule_timeout(sign
 extern signed long schedule_timeout_interruptible(signed long timeout);
 extern signed long schedule_timeout_killable(signed long timeout);
 extern signed long schedule_timeout_uninterruptible(signed long timeout);
-extern void mutex_spin_or_schedule(struct mutex_waiter *waiter, long state,
-                                  unsigned long *flags);
+extern void mutex_spin_or_schedule(struct mutex_waiter *waiter,
+                                  struct task_struct *owner, int cpu);
 asmlinkage void schedule(void);
 
 struct nsproxy;
Index: linux-2.6/kernel/mutex.c
===================================================================
--- linux-2.6.orig/kernel/mutex.c
+++ linux-2.6/kernel/mutex.c
@@ -12,7 +12,8 @@
  *
  *  - Adaptive spinning for mutexes by Peter Zijlstra. (Ported to mainline
  *    from the -rt tree, where it was originally implemented for rtmutexes
- *    by Steven Rostedt, based on work by Gregory Haskins.)
+ *    by Steven Rostedt, based on work by Gregory Haskins, Peter Morreale
+ *    and Sven Dietrich.
  *
  * Also see Documentation/mutex-design.txt.
  */
@@ -157,6 +158,9 @@ __mutex_lock_common(struct mutex *lock, 
        lock_contended(&lock->dep_map, ip);
 
        for (;;) {
+               int cpu = 0;
+               struct task_struct *l_owner;
+
                /*
                 * Lets try to take the lock again - this is needed even if
                 * we get here for the first time (shortly after failing to
@@ -184,8 +188,15 @@ __mutex_lock_common(struct mutex *lock, 
                        return -EINTR;
                }
 
-               mutex_spin_or_schedule(&waiter, state, &flags);
+               __set_task_state(task, state);
+               l_owner = ACCESS_ONCE(lock->owner);
+               if (l_owner)
+                       cpu = task_cpu(l_owner);
+               spin_unlock_mutex(&lock->wait_lock, flags);
+               mutex_spin_or_schedule(&waiter, l_owner, cpu);
+               spin_lock_mutex(&lock->wait_lock, flags);
        }
+       __set_task_state(task, TASK_RUNNING);
 
 done:
        lock_acquired(&lock->dep_map, ip);
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -4600,42 +4600,37 @@ pick_next_task(struct rq *rq, struct tas
        }
 }
 
-#ifdef CONFIG_DEBUG_MUTEXES
-# include "mutex-debug.h"
-#else
-# include "mutex.h"
-#endif
-
-void mutex_spin_or_schedule(struct mutex_waiter *waiter, long state,
-                           unsigned long *flags)
+void mutex_spin_or_schedule(struct mutex_waiter *waiter,
+                           struct task_struct *owner, int cpu)
 {
-       struct mutex *lock = waiter->lock;
        struct task_struct *task = waiter->task;
-       struct task_struct *owner = lock->owner;
+       struct mutex *lock = waiter->lock;
        struct rq *rq;
        int spin = 0;
 
        if (likely(sched_feat(MUTEX_SPIN) && owner)) {
-               rq = task_rq(owner);
+               rq = cpu_rq(cpu);
                spin = (rq->curr == owner);
        }
 
        if (!spin) {
+               preempt_disable();
                schedstat_inc(this_rq(), mtx_sched);
-               __set_task_state(task, state);
-               spin_unlock_mutex(&lock->wait_lock, *flags);
+               preempt_enable();
+
                schedule();
-               spin_lock_mutex(&lock->wait_lock, *flags);
                return;
        }
 
+       preempt_disable();
        schedstat_inc(this_rq(), mtx_spin);
-       spin_unlock_mutex(&lock->wait_lock, *flags);
+       preempt_enable();
+
        for (;;) {
                struct task_struct *l_owner;
 
                /* Stop spinning when there's a pending signal. */
-               if (signal_pending_state(state, task))
+               if (signal_pending_state(task->state, task))
                        break;
 
                /* Mutex got unlocked, try to acquire. */
@@ -4666,7 +4661,6 @@ void mutex_spin_or_schedule(struct mutex
                 */
                cpu_relax();
        }
-       spin_lock_mutex(&lock->wait_lock, *flags);
 }
 
 /*

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to