mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [patch 0/4] futex-pi updates
@ 2006-04-25 16:41 Thomas Gleixner
  2006-04-25 16:41 ` [patch 1/4] rtmutex: Remove buggy BUG_ON in PI boosting code Thomas Gleixner
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Thomas Gleixner @ 2006-04-25 16:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Ingo Molnar

Andrew,

please apply the following updates to the futex-pi code in 2.6.17-rc1-mm3:

- Remove buggy BUG_ON in the PI boosting code
- Enforce waiter bit in owner died situations
- Printk output fixlet
- Add restart handling for interrupted futex_pi lock operations

	tglx

--


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [patch 1/4] rtmutex: Remove buggy BUG_ON in PI boosting code
  2006-04-25 16:41 [patch 0/4] futex-pi updates Thomas Gleixner
@ 2006-04-25 16:41 ` Thomas Gleixner
  2006-04-25 16:41 ` [patch 2/4] futex-pi: Enforce waiter bit when owner died is detected Thomas Gleixner
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2006-04-25 16:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Ingo Molnar, Steven Rostedt

[-- Attachment #1: rtmutex-bugon-fix.patch --]
[-- Type: text/plain, Size: 1571 bytes --]


From: Steven Rostedt <rostedt@goodmis.org>

The condition in that particular BUG_ON can legitimately be the
case, if you have processes A, B, C, D, and E holding the
following locks in this scenario:

 L1 <=blocks= A
               <=owns= L2 <=blocks= B <=owns= L4 <=blocks= D
               <=owns= L3 <=blocks= C <=owns= L5 <=blocks= E

Where the priorities of these tasks are

    B,C < A < D = E

B and C are less than A and A is less than D and E where D and E are
equal (actually it probably works when D and E are not equal too).

As D and E climb the chain, there's a very slight race condition that
could allow for the condition in the offending BUG_ON to be true.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

 kernel/rtmutex.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Index: linux-2.6.17-rc1-mm3/kernel/rtmutex.c
===================================================================
--- linux-2.6.17-rc1-mm3.orig/kernel/rtmutex.c
+++ linux-2.6.17-rc1-mm3/kernel/rtmutex.c
@@ -209,10 +209,8 @@ static int rt_mutex_adjust_prio_chain(ta
 	 * When deadlock detection is off then we check, if further
 	 * priority adjustment is necessary.
 	 */
-	if (!detect_deadlock && waiter->list_entry.prio == task->prio) {
-		BUG_ON(waiter->pi_list_entry.prio != waiter->list_entry.prio);
+	if (!detect_deadlock && waiter->list_entry.prio == task->prio)
 		goto out_unlock_pi;
-	}
 
 	lock = waiter->lock;
 	if (!spin_trylock(&lock->wait_lock)) {

--


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [patch 2/4] futex-pi: Enforce waiter bit when owner died is detected
  2006-04-25 16:41 [patch 0/4] futex-pi updates Thomas Gleixner
  2006-04-25 16:41 ` [patch 1/4] rtmutex: Remove buggy BUG_ON in PI boosting code Thomas Gleixner
@ 2006-04-25 16:41 ` Thomas Gleixner
  2006-04-25 16:41 ` [patch 3/4] rtmutex debug: printk correct task information Thomas Gleixner
  2006-04-25 16:41 ` [patch 4/4] futex-pi: Make use of restart_block when interrupted Thomas Gleixner
  3 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2006-04-25 16:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Ingo Molnar

[-- Attachment #1: futex-pi-enforce-waiter-bit.patch --]
[-- Type: text/plain, Size: 1387 bytes --]


Enforce the waiter bit to be set, when the previous owner has died. This
simplifies the glibc handling of the possible race from userspace tasks
which try to get hold of the lock and cleanup the mess which was leftover
by the unexpectedly died previous owner.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

 kernel/futex.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-2.6.17-rc1-mm3/kernel/futex.c
===================================================================
--- linux-2.6.17-rc1-mm3.orig/kernel/futex.c
+++ linux-2.6.17-rc1-mm3/kernel/futex.c
@@ -1161,11 +1161,16 @@ static int futex_lock_pi(u32 __user *uad
 		 * failed. When the OWNER_DIED bit is set, then we
 		 * know that this is a robust futex and we actually
 		 * take the lock. This is safe as we are protected by
-		 * the hash bucket lock.
+		 * the hash bucket lock. We also set the waiters bit
+		 * unconditionally here, to simplify glibc handling of
+		 * multiple tasks racing to acquire the lock and
+		 * cleanup the problems which were left by the dead
+		 * owner.
 		 */
 		if (curval & FUTEX_OWNER_DIED) {
 			uval = newval;
-			newval = current->pid | FUTEX_OWNER_DIED;
+			newval = current->pid |
+				FUTEX_OWNER_DIED | FUTEX_WAITERS;
 
 			inc_preempt_count();
 			curval = futex_atomic_cmpxchg_inatomic(uaddr,

--


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [patch 3/4] rtmutex debug: printk correct task information
  2006-04-25 16:41 [patch 0/4] futex-pi updates Thomas Gleixner
  2006-04-25 16:41 ` [patch 1/4] rtmutex: Remove buggy BUG_ON in PI boosting code Thomas Gleixner
  2006-04-25 16:41 ` [patch 2/4] futex-pi: Enforce waiter bit when owner died is detected Thomas Gleixner
@ 2006-04-25 16:41 ` Thomas Gleixner
  2006-04-25 16:41 ` [patch 4/4] futex-pi: Make use of restart_block when interrupted Thomas Gleixner
  3 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2006-04-25 16:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Ingo Molnar

[-- Attachment #1: rtmutex-debug-print-current-fix.patch --]
[-- Type: text/plain, Size: 776 bytes --]


Print the information of the current task rather than some random picked
task information.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

 kernel/rtmutex.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.17-rc1-mm3/kernel/rtmutex.c
===================================================================
--- linux-2.6.17-rc1-mm3.orig/kernel/rtmutex.c
+++ linux-2.6.17-rc1-mm3/kernel/rtmutex.c
@@ -180,7 +180,7 @@ static int rt_mutex_adjust_prio_chain(ta
 			prev_max = max_lock_depth;
 			printk(KERN_WARNING "Maximum lock depth %d reached "
 			       "task: %s (%d)\n", max_lock_depth,
-			       task->comm, task->pid);
+			       current->comm, current->pid);
 		}
 		put_task_struct(task);
 

--


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [patch 4/4] futex-pi: Make use of restart_block when interrupted
  2006-04-25 16:41 [patch 0/4] futex-pi updates Thomas Gleixner
                   ` (2 preceding siblings ...)
  2006-04-25 16:41 ` [patch 3/4] rtmutex debug: printk correct task information Thomas Gleixner
@ 2006-04-25 16:41 ` Thomas Gleixner
  3 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2006-04-25 16:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Ingo Molnar, Ulrich Drepper, Jakub Jelinek

[-- Attachment #1: futex-use-restart.patch --]
[-- Type: text/plain, Size: 3431 bytes --]


Make use of restart_block when the lock operation has been interrupted.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

 kernel/futex.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 72 insertions(+), 10 deletions(-)

Index: linux-2.6.17-rc1-mm3/kernel/futex.c
===================================================================
--- linux-2.6.17-rc1-mm3.orig/kernel/futex.c
+++ linux-2.6.17-rc1-mm3/kernel/futex.c
@@ -1077,26 +1077,18 @@ static int futex_wait(u32 __user *uaddr,
  * if there are waiters then it will block, it does PI, etc. (Due to
  * races the kernel might see a 0 value of the futex too.)
  */
-static int futex_lock_pi(u32 __user *uaddr, int detect,
-			 unsigned long sec, long nsec, int trylock)
+static int do_futex_lock_pi(u32 __user *uaddr, int detect, int trylock,
+			    struct hrtimer_sleeper *to)
 {
 	struct task_struct *curr = current;
 	struct futex_hash_bucket *hb;
 	u32 uval, newval, curval;
 	struct futex_q q;
-	struct hrtimer_sleeper timeout, *to = NULL;
 	int ret, attempt = 0;
 
 	if (refill_pi_state_cache())
 		return -ENOMEM;
 
-	if (sec != MAX_SCHEDULE_TIMEOUT) {
-		to = &timeout;
-		hrtimer_init(&to->timer, CLOCK_REALTIME, HRTIMER_ABS);
-		hrtimer_init_sleeper(to, current);
-		to->timer.expires = ktime_set(sec, nsec);
-	}
-
 	q.pi_state = NULL;
  retry:
 	down_read(&curr->mm->mmap_sem);
@@ -1304,6 +1296,76 @@ static int futex_lock_pi(u32 __user *uad
 }
 
 /*
+ * Restart handler
+ */
+static long futex_lock_pi_restart(struct restart_block *restart)
+{
+	struct hrtimer_sleeper timeout, *to = NULL;
+	int ret;
+
+	restart->fn = do_no_restart_syscall;
+
+	if (restart->arg2 || restart->arg3) {
+		to = &timeout;
+		hrtimer_init(&to->timer, CLOCK_REALTIME, HRTIMER_ABS);
+		hrtimer_init_sleeper(to, current);
+		to->timer.expires.tv64 = ((u64)restart->arg1 << 32) |
+			(u64) restart->arg0;
+	}
+
+	pr_debug("lock_pi restart: %p, %d (%d)\n",
+		 (u32 __user *)restart->arg0, current->pid);
+
+	ret = do_futex_lock_pi((u32 __user *)restart->arg0, restart->arg1,
+			       0, to);
+
+	if (ret != -EINTR)
+		return ret;
+
+	restart->fn = futex_lock_pi_restart;
+
+	/* The other values are filled in */
+	return -ERESTART_RESTARTBLOCK;
+}
+
+/*
+ * Called from the syscall entry below.
+ */
+static int futex_lock_pi(u32 __user *uaddr, int detect, unsigned long sec,
+			 long nsec, int trylock)
+{
+	struct hrtimer_sleeper timeout, *to = NULL;
+	struct restart_block *restart;
+	int ret;
+
+	if (sec != MAX_SCHEDULE_TIMEOUT) {
+		to = &timeout;
+		hrtimer_init(&to->timer, CLOCK_REALTIME, HRTIMER_ABS);
+		hrtimer_init_sleeper(to, current);
+		to->timer.expires = ktime_set(sec, nsec);
+	}
+
+	ret = do_futex_lock_pi(uaddr, detect, trylock, to);
+
+	if (ret != -EINTR)
+		return ret;
+
+	pr_debug("lock_pi interrupted: %p, %d (%d)\n", uaddr, current->pid);
+
+	restart = &current_thread_info()->restart_block;
+	restart->fn = futex_lock_pi_restart;
+	restart->arg0 = (unsigned long) uaddr;
+	restart->arg1 = detect;
+	if (to) {
+		restart->arg2 = to->timer.expires.tv64 & 0xFFFFFFFF;
+		restart->arg3 = to->timer.expires.tv64 >> 32;
+	} else
+		restart->arg2 = restart->arg3 = 0;
+
+	return -ERESTART_RESTARTBLOCK;
+}
+
+/*
  * Userspace attempted a TID -> 0 atomic transition, and failed.
  * This is the in-kernel slowpath: we look up the PI state (if any),
  * and do the rt-mutex unlock.

--


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-04-25 16:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-04-25 16:41 [patch 0/4] futex-pi updates Thomas Gleixner
2006-04-25 16:41 ` [patch 1/4] rtmutex: Remove buggy BUG_ON in PI boosting code Thomas Gleixner
2006-04-25 16:41 ` [patch 2/4] futex-pi: Enforce waiter bit when owner died is detected Thomas Gleixner
2006-04-25 16:41 ` [patch 3/4] rtmutex debug: printk correct task information Thomas Gleixner
2006-04-25 16:41 ` [patch 4/4] futex-pi: Make use of restart_block when interrupted Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®