On Mon, 8 Dec 2008, Andreas Bombe wrote:
> Apart from sleep_on() calls that could be easily converted to
> wait_event() and completion calls amiflop also used a flag in ms_delay()
> and ms_isr() as a custom mutex for ms_delay() without a need for
> explicit unlocking. I converted that to a standard mutex semaphore.
>
> The replacement for the unconditional sleep_on() in fd_motor_on() is a
> complete_all() together with a INIT_COMPLETION() before the mod_timer()
> call. It appears to me that fd_motor_on() might be called concurrently
> and fd_select() does not guarantee mutual exclusivity in the case the
> same drive gets selected again.
>
> Signed-off-by: Andreas Bombe <[EMAIL PROTECTED]>
> ---
>
> Note: I haven't tested these changes beyond correct compilation.
>
> The complete_all() with INIT_COMPLETION() was because I'm not sure about
> whether fd_motor_on() might run concurrently. Better safe than sorry.
I'm afraid my Amiga floppy drive doesn't work anymore...
Any other testers available?
> --- a/drivers/block/amiflop.c
> +++ b/drivers/block/amiflop.c
> @@ -220,19 +218,17 @@ static irqreturn_t ms_isr(int irq, void *dummy)
> A more generic routine would do a schedule a la timer.device */
> static void ms_delay(int ms)
> {
> - unsigned long flags;
> int ticks;
> + static DECLARE_MUTEX(mutex);
^^^^^^^^^^^^^
Shouldn't this be a real mutex (declared using DEFINE_MUTEX(mutex)) instead of
a semaphore used as a mutex?
> +
> if (ms > 0) {
> - local_irq_save(flags);
> - while (ms_busy == 0)
> - sleep_on(&ms_wait);
> - ms_busy = 0;
> - local_irq_restore(flags);
> + down(&mutex);
^^^^
mutex_lock
> ticks = MS_TICKS*ms-1;
> ciaa.tblo=ticks%256;
> ciaa.tbhi=ticks/256;
> ciaa.crb=0x19; /*count eclock, force load, one-shoot, start */
> - sleep_on(&ms_wait);
> + wait_for_completion(&ms_wait_completion);
> + up(&mutex);
mutex_unlock
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
