2002-02-12 20:01:24

by Bob Miller

[permalink] [raw]
Subject: [PATCH] 2.5.4 Can't use spin_lock_* with wait_queue_head_t object.

There is code in sched.c that uses the spin_lock_* interfaces to acquire and
release the lock in the wait_queue_head_t embedded in the struct completion.

This is wrong and causes the system to OPPs on startup when compiled with
wait.h:USE_RW_WAIT_QUEUE_SPINLOCK set to 1. The patch below changes the
spin_lock_* calls to wq_write_lock_*.


--
Bob Miller Email: [email protected]
Open Software Development Lab Phone: 503.626.2455 Ext. 17



diff -ru linux.2.5.4-orig/include/linux/wait.h linux.2.5.4/include/linux/wait.h
--- linux.2.5.4-orig/include/linux/wait.h Sun Feb 10 17:50:07 2002
+++ linux.2.5.4/include/linux/wait.h Mon Feb 11 11:22:21 2002
@@ -45,6 +45,7 @@
# define wq_read_unlock read_unlock
# define wq_write_lock_irq write_lock_irq
# define wq_write_lock_irqsave write_lock_irqsave
+# define wq_write_unlock_irq write_unlock_irq
# define wq_write_unlock_irqrestore write_unlock_irqrestore
# define wq_write_unlock write_unlock
#else
@@ -57,6 +58,7 @@
# define wq_read_unlock_irqrestore spin_unlock_irqrestore
# define wq_write_lock_irq spin_lock_irq
# define wq_write_lock_irqsave spin_lock_irqsave
+# define wq_write_unlock_irq spin_unlock_irq
# define wq_write_unlock_irqrestore spin_unlock_irqrestore
# define wq_write_unlock spin_unlock
#endif
diff -ru linux.2.5.4-orig/kernel/sched.c linux.2.5.4/kernel/sched.c
--- linux.2.5.4-orig/kernel/sched.c Sun Feb 10 17:50:12 2002
+++ linux.2.5.4/kernel/sched.c Mon Feb 11 11:22:21 2002
@@ -806,15 +806,15 @@
{
unsigned long flags;

- spin_lock_irqsave(&x->wait.lock, flags);
+ wq_write_lock_irqsave(&x->wait.lock, flags);
x->done++;
__wake_up_common(&x->wait, TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 1, 0);
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ wq_write_unlock_irqrestore(&x->wait.lock, flags);
}

void wait_for_completion(struct completion *x)
{
- spin_lock_irq(&x->wait.lock);
+ wq_write_lock_irq(&x->wait.lock);
if (!x->done) {
DECLARE_WAITQUEUE(wait, current);

@@ -822,14 +822,14 @@
__add_wait_queue_tail(&x->wait, &wait);
do {
__set_current_state(TASK_UNINTERRUPTIBLE);
- spin_unlock_irq(&x->wait.lock);
+ wq_write_unlock_irq(&x->wait.lock);
schedule();
- spin_lock_irq(&x->wait.lock);
+ wq_write_lock_irq(&x->wait.lock);
} while (!x->done);
__remove_wait_queue(&x->wait, &wait);
}
x->done--;
- spin_unlock_irq(&x->wait.lock);
+ wq_write_unlock_irq(&x->wait.lock);
}

#define SLEEP_ON_VAR \


2002-02-12 21:22:12

by Richard B. Johnson

[permalink] [raw]
Subject: Re: [PATCH] 2.5.4 Can't use spin_lock_* with wait_queue_head_t object.

On Tue, 12 Feb 2002, Bob Miller wrote:

> There is code in sched.c that uses the spin_lock_* interfaces to acquire and
> release the lock in the wait_queue_head_t embedded in the struct completion.
>
Isn't it just that the spin_lock wasn't initialized at the start?


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (797.90 BogoMips).

I was going to compile a list of innovations that could be
attributed to Microsoft. Once I realized that Ctrl-Alt-Del
was handled in the BIOS, I found that there aren't any.


2002-02-12 22:16:01

by Bob Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.5.4 Can't use spin_lock_* with wait_queue_head_t object.

On Tue, Feb 12, 2002 at 04:23:39PM -0500, Richard B. Johnson wrote:

> On Tue, 12 Feb 2002, Bob Miller wrote:
>
> > There is code in sched.c that uses the spin_lock_* interfaces to acquire and
> > release the lock in the wait_queue_head_t embedded in the struct completion.
> >
> Isn't it just that the spin_lock wasn't initialized at the start?
>

No. The DECLARE_COMPLETION() macro was called in the case that oops'd.

The problem is that when wait.h:USE_RW_WAIT_QUEUE_SPINLOCK is set
the type of lock in the wait_queue_head_t changes from being a
spinlock_t to a rwlock_t.

If you also set CONFIG_DEBUG_SPINLOCK, the wq_lock_t in the
wait_queue_head_t is initialized with a rwlock_t magic number
(0xdeaf1eed), but the spin_lock_XXX() code checks for the spinlock_t
magic number (0xdead4ead) and calls BUG() when the check fails.


--
Bob Miller Email: [email protected]
Open Software Development Lab Phone: 503.626.2455 Ext. 17