lkml.org 
[lkml]   [2005]   [Aug]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Possible race in sound/oss/forte.c ?
On 8/11/05, John M. King <jmking1@uiuc.edu> wrote:
> I know the OSS drivers are deprecated, but I'm trying to figure this out
> for my own understanding.
>
> Here's code from sound/oss/forte.c, in the write system call handler. A
> test has already been performed (under the protection of the lock) and
> the driver has decided to sleep.
>
> add_wait_queue (&channel->wait, &wait);
>
> for (;;) {
> spin_unlock_irqrestore (&chip->lock, flags);
>
> set_current_state (TASK_INTERRUPTIBLE);
> schedule();
>
> spin_lock_irqsave (&chip->lock, flags);
>
> if (channel->frag_num - channel->filled_frags)
> break;
> }
>
> remove_wait_queue (&channel->wait, &wait);
> set_current_state (TASK_RUNNING);
>
> The driver's interrupt handler calls wake_up_all(). What if an
> interrupt occurs just after the spin_unlock_irqrestore() but before
> setting TASK_INTERRUPTIBLE (and the interrupt handler does stuff that
> causes the tested conditional to be true as well)? The interrupt calls
> wake_up_all(), but then when control returns here, the process will mark
> itself TASK_INTERRUPTIBLE right away and sleep, effectively missing the
> wake_up_all().
>
> Is this a race condition? If not, can someone point out the error(s) in
> my reasoning? Please CC me as I'm not subscribed to the list.

This is broken code. You are supposed to set the state before adding
oneself to the wait-queue, if you are going to unconditionally sleep,
I believe.

So, the loop probably should be:

prepare_to_wait(&channel->wait, &wait, TASK_INTERRUPTIBLE);

for (;;) {
spin_unlock_irqrestore (&chip->lock, flags);

set_current_state (TASK_INTERRUPTIBLE); // redundant in the
first iteration
schedule();

spin_lock_irqsave (&chip->lock, flags);

if (channel->frag_num - channel->filled_frags)
break;
}

finish_wait(&channel->wait, &wait);

Thanks,
Nish
-
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/

\
 
 \ /
  Last update: 2005-08-11 23:33    [W:0.037 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site