Messages in this thread Patch in this message | | | Subject | Races in sleep_on() | From | David Woodhouse <> | Date | Thu, 22 Jan 2004 12:57:52 +0000 |
| |
On Thu, 2004-01-22 at 00:54 -0800, Andrew Morton wrote: > > 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 ===== --- 1.7/include/linux/smp_lock.h Mon Jan 19 23:38:11 2004 +++ edited/include/linux/smp_lock.h Thu Jan 22 09:15:17 2004 @@ -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 ===== --- 1.236/kernel/sched.c Mon Jan 19 23:38:12 2004 +++ edited/kernel/sched.c Thu Jan 22 10:49:55 2004 @@ -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;
-- dwmw2
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |