Messages in this thread | | | Date | Wed, 10 Sep 2003 20:58:31 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH] blk API update (and bug fix) to CDU535 cdrom driver |
| |
This cli-sti removal seems exactly as broken as all the ones i've NAKed on kernel-janitors. There's no evidence that I can see for locking in the interrupt handler. Here's a race example:
CPU 1 CPU 2 sony_sleep(); spin_lock_irq(&sonycd535_lock); enable_interrupts(); cdu535_interrupt(); disable_interrupts(); if (waitqueue_active(&cdu535_irq_wait)) {
prepare_to_wait(&cdu535_irq_wait, &wait, TASK_INTERRUPTIBLE); spin_unlock_irq(&sonycd535_lock);
Either you need to move prepare_to_wait before enable_interrupts or grab the sonycd535_lock in interrupt context.
Hang on a minute. This driver is always polled, and never interrupt driven. There's no problem because this code path is never executed :-P Nevertheless, it's probaby worth fixing so other more complex drivers (eg cdu31a) don't copy it wrongly.
BTW, I bet sony_sleep() shouldn't be calling the new-and-improved yield().
-- "It's not Hollywood. War is real, war is primarily not about defeat or victory, it is about death. I've seen thousands and thousands of dead bodies. Do you think I want to have an academic debate on this subject?" -- Robert Fisk - 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/
| |