Hello!
> #2 crash be explained via any of the bugs you fixed? (i.e. memory
> corruption?)
Yes, I found the reason, it is really fixed by taking tasklist_lock.
This happens after task struct with not cleared pi_state_list is freed
and the list of futex_pi_state's is corrupted.
Meanwhile... two more bugs were found.
The first chunk: results in self-inflicted deadlock inside glibc.
Sometimes futex_lock_pi returns -ESRCH, when it is not expected
and glibc enters to for(;;) sleep() to simulate deadlock. This problem
is quite obvious and I think the patch is right. Though it looks like
each "if" in futex_lock_pi() got some stupid special case "else if". :-)
The second chunk: sometimes futex_lock_pi() returns -EDEADLK,
when nobody has the lock. The reason is also obvious (see comment
in the patch), but correct fix is far beyond my comprehension.
I guess someone already saw this, the chunk:
if (rt_mutex_trylock(&q.pi_state->pi_mutex))
ret = 0;
is obviously from the same opera. But it does not work, because the
rtmutex is really taken at this point: wake_futex_pi() of previous
owner reassigned it to us. My fix works. But it looks very stupid.
I would think about removal of shift of ownership in wake_futex_pi()
and making all the work in context of process taking lock.
Both bugs show up when running glibc's tst-robustpi8 long enough.
Yes, all this about pre May 8 futexes. Seems, updates did not change
anything in logic, but I am not sure.
--- kernel/futex.c.intermediate 2007-05-23 14:48:27.000000000 +0400
+++ kernel/futex.c 2007-05-23 14:58:06.000000000 +0400
@@ -1244,8 +1244,21 @@ static int futex_lock_pi(u32 __user *uad
if (unlikely(curval != uval))
goto retry_locked;
ret = 0;
- }
- goto out_unlock_release_sem;
+ } else if (ret == -ESRCH) {
+ /* Process could exit right now, so that robust list
+ * was processed after we got uval. Retry. */
+ pagefault_disable();
+ curval = futex_atomic_cmpxchg_inatomic(uaddr,
+ uval, uval);
+ pagefault_enable();
+ if (unlikely(curval == -EFAULT))
+ goto uaddr_faulted;
+ if (unlikely(curval != uval)) {
+ printk("RETRY %x %x %x\n", current->pid, uval,
curval);
+ goto retry_locked;
+ }
+ }
+ goto out_unlock_release_sem;
}
/*
@@ -1361,6 +1374,22 @@ static int futex_lock_pi(u32 __user *uad
if (ret && q.pi_state->owner == curr) {
if (rt_mutex_trylock(&q.pi_state->pi_mutex))
ret = 0;
+ /* Holy crap... Now futex lock returns -EDEADLK
+ * sometimes, because ownership was passed to us while
+ * unlock of previous owner. Who wrote this?
+ * Please, fix this correctly. For now:
+ */
+ if (ret == -EDEADLK) {
+ pagefault_disable();
+ uval = futex_atomic_cmpxchg_inatomic(uaddr,
+ 0, 0);
+ pagefault_enable();
+ if (uval != -EFAULT &&
+ (uval&FUTEX_TID_MASK) == current->pid) {
+ printk("ALERT1 %x\n", uval);
+ ret = 0;
+ }
+ }
}
/* Unqueue and drop the lock */
unqueue_me_pi(&q, hb);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
