Re: [PATCH] blk API update (and bug fix) to CDU535 cdrom driver

[Matthew Wilcox]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/