Messages in this thread | | | Date | Thu, 11 Aug 2005 14:31:07 -0700 | From | Nish Aravamudan <> | Subject | Re: 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/
| |