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(¤t->pi_lock);
- adjust_prio(current);
+ adjust_prio(current,0);
_raw_spin_unlock(¤t->pi_lock);
}
@@ -1453,7 +1463,7 @@ static int __sched down_rtsem(struct rt_
* PI boost has to go
*/
_raw_spin_lock(¤t->pi_lock);
- adjust_prio(current);
+ adjust_prio(current,0);
_raw_spin_unlock(¤t->pi_lock);
}
trace_unlock_irqrestore(&tracelock, flags);
- Follow-Ups:
- Re: 2.6.15-rt17
- From: Steven Rostedt <[email protected]>
- Re: 2.6.15-rt17
- Prev by Date: Re: GFS2 Filesystem [14/16]
- Next by Date: Re: [PATCH] Define wc_wmb, a write barrier for PCI write combining
- Previous by thread: Re: 2.6.15-rt17
- Next by thread: Re: 2.6.15-rt17
- Index(es):
 |