wait_event and interrupts

[Kalle Pokki]

Rune Torgersen wrote:

>Hi
>I have a driver that roughly does something like:
>
>int driver_read(int cs, int addr, void *buf, int len)
>{
>    hw_done = 0;
>    /* init_hw */
>
>    if (!hw_done)
>    {
>        ret = wait_event_interruptible_timeout(inq, hw_done, TIMEOUT);
>        if (ret == 0)
>        {
>            if (hw_done)
>                goto hw_finished;
>			
>            return -EIO;
>        }
>    }
>hw_finished:
>    return len;
>}
>
>static irqreturn_t myinterrupt(int irq, void * dev_id, struct pt_regs *
>regs)
>{
>    hw_done = 1;
>    schedule_work(&tqueue);
>
>    return IRQ_HANDLED;
>}
>
>static void do_softint(void *private_)
>{
>    wake_up_interruptible(&inq);
>
>}
>
>I have a problem however with this, because in about 10% of my cases,
>the interrupt triggers very fast, and ends up being served between the
>check for hw_done and the wait_event call. This cause the wait to
>timeout instead of getting waked up.
>
>Is there a better way of doing this?
>I do not want to do a busy wait, because the hardware can take up to
>several 100's of ms to return, but most often returns within 20us.
>  
>
You seem to create an unnecessary race condition by checking the hw_done 
variable in the read function. You can avoid this by changing it to

int driver_read(int cs, int addr, void *buf, int len)
{
    hw_done = 0;
    /* init_hw */

    if (wait_event_interruptible_timeout(inq, hw_done, TIMEOUT))
	return -ERESTARTSYS;

    hw_done = 0;

    /* copy the data to user space here */

    return len;
}

The wait_event_interruptible_timeout() function checks the variable for 
you. You still must make sure the read function does the right thing if 
it is called simultaneously by more than one application.