Re: 2.6.15-rt17

[Esben Nielsen <simlo@phys.au.dk>]

On Thu, 23 Feb 2006, Thomas Gleixner wrote:

> On Wed, 2006-02-22 at 17:50 +0100, Esben Nielsen wrote:
> > I didn't know anyone looked at my patch! I am ofcourse happy about it :-)
>
> Was just delayed due to other work in progress.
>

You didn't use the "TestRTMutex" I send along with the patch :-(

Since I learned to use unittesting that way I have been a big fan. It does
catch a lot of stupid bugs without having to wait for the program to get
there while keeping the ability to debug with gdb and fix it right away.
And most important: you can keep the tests and check if your program still
parses them after a rewrite. Very usefull to prevent repeating bugs.

So here is the issues I have found:

1) grablockBKL.tst failes. Apparently you can "grab" the BKL now? Is this
intended? I have updated the test to accept this.

2) 2locksdeadlock.tst loops forever. It is a livelock: When two RT-tasks
"deadlocks" on two mutexes they keep waking up each other in other. I
quickly fixed that bug.

3) While fixing that I came to think about what happens when you signal a
task blocked on a task blocked on a task. I thus wrote
2lock3tasksBoostSignal.tst. Well, the priorities wasn't set back
correctly. I fixed that too.

I have attached the patch againt 2.6.17-rt17 (and therefore also
included the previous patch) along with the updated tester and tests.

Esben


> > That was why I had _reversed_ the lock ordering relative to normal in the
> > original patch: First lock task->pi_lock. Assign lock. Lock
> > lock->wait_lock. Then unlock task->pi_lock. Now it is safe to refer to
> > lock. To avoid deadlocks I used _raw_spin_trylock() when locking the 2.
> > lock.
>
> Stupid me. I messed that one up. Should show up in the next -rt
>
> Thanks
>
> 	tglx
>
>

Attachment: TestRTMutex.tgz
Description: GNU Zip compressed data

--- linux-2.6.15-rt17/kernel/rt.c.orig	2006-02-22 16:53:44.000000000 +0100
+++ linux-2.6.15-rt17/kernel/rt.c	2006-02-25 17:49:53.000000000 +0100
@@ -873,10 +873,19 @@ pid_t get_blocked_on(task_t *task)
 	}
 
 	lock = task->blocked_on->lock;
+	
+	/* 
+	 * Now we have to take lock->wait_lock _before_ releasing
+	 * task->pi_lock. Otherwise lock can be deallocated while we are
+	 * refering to it as the subsystem has no way of knowing about us
+	 * hanging around in here.
+	 */
+	if (!_raw_spin_trylock(&lock->wait_lock)) {
+		_raw_spin_unlock(&task->pi_lock);
+		goto try_again;
+        }
 	_raw_spin_unlock(&task->pi_lock);
 
-	if (!_raw_spin_trylock(&lock->wait_lock))
-		goto try_again;
 
 	owner = lock_owner(lock);
 	if (owner)
@@ -964,14 +973,24 @@ static inline int calc_pi_prio(task_t *t
 }
 
 /*
- * Adjust priority of a task
+ * Adjust priority of a task. 
+ * If wakeup is set it will be woken when blocked on a lock when adjusted
  */
-static void adjust_prio(task_t *task)
+static void adjust_prio(task_t *task, int wakeup)
 {
 	int prio = calc_pi_prio(task);
 
-	if (task->prio != prio)
+	if (task->prio != prio) {
 		mutex_setprio(task, prio);
+		if (wakeup && task->blocked_on) {
+                        /*
+			 * The owner will have the blocked field set if it is
+			 * blocked on a lock. So in this case we want to wake
+			 * the owner up so it can boost who it is blocked on.
+			 */
+			wake_up_process_mutex(task);
+		}
+	}
 }
 
 /*
@@ -1034,16 +1053,7 @@ static long task_blocks_on_lock(struct r
 
 	/* Add the new top priority waiter to the owners waiter list */
 	plist_add(&waiter->pi_list, &owner->pi_waiters);
-	adjust_prio(owner);
-
-	/*
-	 * The owner will have the blocked field set if it is
-	 * blocked on a lock. So in this case we want to wake
-	 * the owner up so it can boost who it is blocked on.
-	 */
-	if (owner->blocked_on)
-		wake_up_process_mutex(owner);
-
+	adjust_prio(owner,1);
 	_raw_spin_unlock(&owner->pi_lock);
 	return ret;
 }
@@ -1139,7 +1149,7 @@ static void remove_waiter(struct rt_mute
 			next = lock_first_waiter(lock);
 			plist_add(&next->pi_list, &owner->pi_waiters);
 		}
-		adjust_prio(owner);
+		adjust_prio(owner,1);
 		_raw_spin_unlock(&owner->pi_lock);
 	}
 }
@@ -1201,7 +1211,7 @@ static void release_lock(struct rt_mutex
 
 	/*  Readjust priority, when necessary. */
 	_raw_spin_lock(&current->pi_lock);
-	adjust_prio(current);
+	adjust_prio(current,0);
 	_raw_spin_unlock(&current->pi_lock);
 }
 
@@ -1453,7 +1463,7 @@ static int __sched down_rtsem(struct rt_
 		 * PI boost has to go
 		 */
 		_raw_spin_lock(&current->pi_lock);
-		adjust_prio(current);
+		adjust_prio(current,0);
 		_raw_spin_unlock(&current->pi_lock);
 	}
 	trace_unlock_irqrestore(&tracelock, flags);