Re: 2.6.15-rt17

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);

Relevant Pages

  • [PATCH] sysfs: fix deadlock
    ... i've done the v3 patch below - that seems to have passed all my testing ... without any new bugs found. ... Here is a basic fix for the sysfs lor. ... proactive methods to widen our lock hierarchy knowledge that would be ...
    (Linux-Kernel)
  • Re: Accessing address space of a process through kld!!
    ... (I found that bugs from theoretical understanding how everything works). ... references np after releasing lock on head of corresponding PATRICIA ... which protects these structures. ...
    (freebsd-hackers)
  • Re: 2.6.15-rt17
    ... Very usefull to prevent repeating bugs. ... I have attached the patch againt 2.6.17-rt17 (and therefore also ... lock. ... To avoid deadlocks I used _raw_spin_trylockwhen locking the 2. ...
    (Linux-Kernel)
  • radeon driver causes random lockups
    ... It's been causing my laptop to lock up randomly. ... Please update the following bugs if you are experiencing this: ...
    (Fedora)
  • Re: 2.6.15-rt17
    ... attached is the test and a better patch which solves the problem. ... lock. ... * Adjust priority of a task ... * Adjust priority of a task and wake it up if the prio is changed ...
    (Linux-Kernel)