On 8/19/05, Alan Cox <[EMAIL PROTECTED]> wrote: > On Gwe, 2005-08-19 at 10:13 +0200, Peter T. Breuer wrote: > > The following "sleep under spinlock" is still present as of linux > > 2.6.12.5 in sound/oss/sequencer.c in midi_outc: > > > > > > n = 3 * HZ; /* Timeout */ > > > > spin_lock_irqsave(&lock,flags); > > while (n && !midi_devs[dev]->outputc(dev, data)) { > > interruptible_sleep_on_timeout(&seq_sleeper, HZ/25); > > n--; > > } > > spin_unlock_irqrestore(&lock,flags); > > > > > > I haven't thought about it, just noted it. It's been there forever > > (some others in the sound architecture have been gradually disappearing > > as newer kernels come out). > > Yep thats a blind substition of lock_kernel in an old tree it seems. > Probably my fault. Should drop it before the sleep and take it straight > after.
Also, the use of n makes no sense. Indicates total sleep for 3 seconds, but actually sleep for 40 milliseconds 3*HZ times (potentially)? In any case, probably should be: timeout = jiffies + 3*HZ; spin_lock_irqsave(&lock, flags); while (time_before(jiffies, timeout) && !midi_devs[dev]->outputc(dev, data)) { spin_unlock_irqrestore(&lock, flags); interruptible_sleep_on_timeout(&seq_sleeper, msecs_to_jiffies(40)); spin_lock_irqsave(&lock, flags); } spin_lock_irqrestore(&lock, flags); Or something similar.... If those locks weren't there, we could use wait_event_interruptible_timeout(). Should we create a locked version? Thanks, Nish - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/