Re: [patch] Real-Time Preemption, -RT-2.6.9-rc4-mm1-U5

[Ingo Molnar]

* Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:

> On Tue, 2004-10-19 at 11:04, Ingo Molnar wrote:
> > > All sleep_on variants trigger the irqs_disabled() check in schedule(). 
> > > tglx
> > 
> > ah, forgot that the waitqueue lock is a raw lock. Is there _any_
> > scenario where sleep_on() is actually correct kernel code?
> 
> Hmm, the sleep_on() variants are used quite a lot over the kernel.
> Whats wrong with them and to what should they be converted ?

they are racy on SMP. It does:

	current->state = TASK_INTERRUPTIBLE;

	schedule();

which is almost always a bug to go to sleep via sleep_on() _after_
checking for the condition, because the following could happen:

	CPU1				CPU2

	if (condition)
		goto done;

					wake_up(&waitqueue);

	current->state = TASK_INTERRUPTIBLE;

	schedule();

The proper interface is wait_event() (and variants).

your patch probably only works due to timing - the wakeup always happens
after sleep_on() has been called.

this particular NFS case is probably only correct due to userspace
behavior. The code is apparently relying on the wake_up() never
happening _before_ we do the sleep_on().

so, could you try the init_MUTEX_LOCKED() fix plus the patch below -
does that turn off the deadlock assert? (Plus also uncomment the
RWSEM_BUG() around line 130.)

	Ingo

--- linux/lib/rwsem-generic.c.orig
+++ linux/lib/rwsem-generic.c
@@ -750,6 +750,15 @@ void fastcall sema_init(struct semaphore
 	case 0:
 		init_rwsem(&sem->lock);
 		down(sem);
+#ifdef CONFIG_RWSEM_DEADLOCK_DETECT
+		{
+			unsigned long flags;
+
+			rwsem_lock_irqsave(&rwsem_lock, flags);
+			rwsem_owner_del(&sem->lock);
+			rwsem_unlock_irqrestore(&rwsem_lock, flags);
+		}
+#endif
 		break;
 	default:
 		RWSEM_BUG();
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/