Races in sleep_on()

PATCH David Woodhouse 1 participant 1 message Jan 22, 2004 7 messages not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

David Woodhouse

author

2004-01-22 12:58 UTC #7 lore raw

Quoted 8 lines · Andrew Morton

Quoted 2 lines
Yes. That's why I advocated the BUG_ON(!BKL) a _long_ time ago. Arjan had a huge list of bogus callers even in 2.4.
So... do me a patch which does a WARN_ON()? Make sure that it shuts itself up after the first five messages so people don't hate us too much.
To get best coverage you'll need to make the !CONFIG_SMP&&!CONFIG_PREEMPT version of lock_kernel() set a flag or something.

Since the spinlock is a NOP anyway there doesn't seem to be any harm in just letting the UP version use the same code as the SMP/PREEMPT versions.

I've done the _timeout versions too, despite the frequent whinge that 'it's OK if we miss wakeups because we time out eventually anyway'. That argument was never acceptable to me anyway.

===== include/linux/smp_lock.h 1.7 vs edited =====

include/linux/smp_lock.h +3 -1

@@ -5,7 +5,9 @@
 #include <linux/sched.h>
 #include <linux/spinlock.h>
 
-#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
+#define BKL_DEBUG /* For testing for sleep_on() abuse */
+
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT) || defined(BKL_DEBUG)
 
 extern spinlock_t kernel_flag;

===== kernel/sched.c 1.236 vs edited =====

kernel/sched.c +17 -0

@@ -1903,10 +1903,21 @@
 	__remove_wait_queue(q, &wait);			\
 	spin_unlock_irqrestore(&q->lock, flags);
 
+#define SLEEP_ON_BKLCHECK				\
+	if (unlikely(!kernel_locked()) &&		\
+	    sleep_on_bkl_warnings < 10) {		\
+		sleep_on_bkl_warnings++;		\
+		WARN_ON(1);				\
+	}
+
+static int sleep_on_bkl_warnings;
+
 void interruptible_sleep_on(wait_queue_head_t *q)
 {
 	SLEEP_ON_VAR
 
+	SLEEP_ON_BKLCHECK
+
 	current->state = TASK_INTERRUPTIBLE;
 
 	SLEEP_ON_HEAD

@@ -1920,6 +1931,8 @@
 {
 	SLEEP_ON_VAR
 
+	SLEEP_ON_BKLCHECK
+
 	current->state = TASK_INTERRUPTIBLE;
 
 	SLEEP_ON_HEAD

@@ -1935,6 +1948,8 @@
 {
 	SLEEP_ON_VAR
 
+	SLEEP_ON_BKLCHECK
+
 	current->state = TASK_UNINTERRUPTIBLE;
 
 	SLEEP_ON_HEAD

@@ -1947,6 +1962,8 @@
 long sleep_on_timeout(wait_queue_head_t *q, long timeout)
 {
 	SLEEP_ON_VAR
+
+	SLEEP_ON_BKLCHECK
 
 	current->state = TASK_UNINTERRUPTIBLE;

Signature · 1 line

dwmw2