Safely giving up a lock before sleeping

Safely giving up a lock before sleeping

[Russ Lewis]

I'm writing a module where I need to go to sleep and release a spinlock
as I do so.  I know how to do this, but it's not elegant...I'm wondering
if anyone has a more elegant solution to what I would guess is a common
problem.


The problem:
I have a module that gets called by other modules' bottom half
handlers.  These calls add information to a job queue I manage.  I then
have user processes that perform read operations on device files asking
for work.  If there is no work currently available, I put them to
sleep.  Of course, this user thread must release the spinlock before
going to sleep.  The calls from the bottom half handlers wake up the
wait queue after they put new work on the job queue.

If I implement this by calling spin_unlock_irqrestore() immediately
followed by interruptible_sleep_on(), then I have a race condition where
I could release the lock and immediately have a bottom half handler on
another processor grab it, put data in the queue, and wake the wait
queue.  My original (user-side) process then happily goes to sleep,
unaware that new information is available.

My current solution:
I looked at the source of interruptible_sleep_on and stuck my
spin_unlock_irqsave right into the middle of it.  My code currently is:

current->state = TASK_INTERRUPTIBLE;
add_wait_queue_exclusive(...);
spin_unlock_irqrestore(...);
schedule();

This avoids the wait condition, since the lock is not released until the
process is set to INTERRUPTIBLE and is on the qait queue.  Thus, if the
other process races me and puts work on the queue and wakes up the wait
queue before I complete schedule(), then my state is simply reset to
TASK_RUNNING and the user process just keeps going.

Of course, since I need the lock to do anything, the first thing I do
(after removing myself from the wait queue and checking for signals) is
to relock the lock.

Is this a common problem?  Is there a more elegant solution to this?

Re: Safely giving up a lock before sleeping

[Richard B. Johnson]

On Mon, 23 Jul 2001, Russ Lewis wrote:
[SNIPPED...]

> Of course, since I need the lock to do anything, the first thing I do
> (after removing myself from the wait queue and checking for signals) is
> to relock the lock.
> 
> Is this a common problem?  Is there a more elegant solution to this?
> 

If you need to protect a piece of code, and the variables it
accesses are ONLY accessed by that code, investigate the use
of a simple spin-lock (no disabling interrupts). This can
prevent races such as you describe.

	spin_lock_irqsave(&big_lock, flags);
	do_critical_stuff();    /* Where global variables could change */
	spin_lock(&little_lock);  /* Simple spin for any other entry */
	spin_unlock_irqrestore(&big_lock, flags);  /* Interrupts now on */
	do_less_critical_stuff_including_sleep();
	spin_unlock(&little_lock);

Note that this turns a possible race into a possible CPU time-eating
spin so you need to carefully look at how the code is written. You
could turn that spin-wait into a sleep if you used a semaphore to
protect that section of code "up(), and down()".


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 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.

Re: Safely giving up a lock before sleeping

[Manfred Spraul]

> If I implement this by calling spin_unlock_irqrestore() immediately
> followed by interruptible_sleep_on(), then I have a race condition
> where I could release the lock and immediately have a bottom half
> handler on another processor grab it, put data in the queue, and wake
> the wait queue.  My original (user-side) process then happily goes
> to sleep, unaware that new information is available.

DO NOT use sleep_on in new code.
The correct replacement is wait_event() in <linux/wait.h> if the
spinlock is a normal (i.e. non-irq) spinlock.

Your spinlock is probably a spin_lock_irq() or spin_lock_bh() lock, then
you must build your own wait_event() wrapper.
check <linux/raid/md_k.h> for a wrapper with spin_lock_irq
(wait_event_lock_irq)

The mouse driver sample in linux/documentation also explains the correct
way to release a lock and schedule.

--
    Manfred