Re: [PATCH] ataflop: unlock ataflop_probe_lock at atari_floppy_init()

Michael Schmitz
2021-10-17T01:52:20Z

Hi Tetsuo,

thank you for fixing this bug!

On 17/10/21 02:25, Tetsuo Handa wrote:
> Commit bf9c0538e485b591 ("ataflop: use a separate gendisk for each media
> format") introduced ataflop_probe_lock mutex, but forgot to unlock the
> mutex when atari_floppy_init() (i.e. module loading) succeeded. If
> ataflop_probe() is called, it will deadlock on ataflop_probe_lock mutex.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: bf9c0538e485b591 ("ataflop: use a separate gendisk for each media format")
> ---
> To m68k users
>
> This patch suggests that nobody is testing this module using a real hardware.

Not as a module, no. I use the Atari floppy driver built-in. Latest
kernel version I ran was 5.13.

Relevant kernel log excerpt:

calling atari_floppy_init+0x0/0x4d4 @ 1
Atari floppy driver: max. HD, track buffering
Probing floppy drive(s):
fd0
initcall atari_floppy_init+0x0/0x4d4 returned 0 after 675082 usecs

Haven't tried to read from the drive in a while though... waiting for
floppy I/O isn't my favourite spectator sport.

I take it a read attempt should fail, without your patch?

> Can somebody test this module?

Yes.

> Is current m68k hardware still supporting Atari floppy?

Yes.

Cheers,

Michael Schmitz

> If Atari floppy is no longer supported, do we still need this module?
>
> To Christoph Hellwig and Luis Chamberlain
>
> If we move __register_blkdev() in atari_floppy_init() to the end of
> atari_floppy_init() and move unregister_blkdev() in atari_floppy_exit() to
> the beginning of atari_floppy_exit(), we can remove unregister_blkdev() from
> atari_floppy_init(), and I think we can also remove ataflop_probe_lock mutex
> because probe function and __exit function are serialized by major_names_lock
> mutex.
>
> drivers/block/ataflop.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
> index a093644ac39f..39b42cb8d173 100644
> --- a/drivers/block/ataflop.c
> +++ b/drivers/block/ataflop.c
> @@ -2072,7 +2072,8 @@ static int __init atari_floppy_init (void)
> UseTrackbuffer ? "" : "no ");
> config_types();
>
> - return 0;
> + ret = 0;
> + goto out_unlock;
>
> err:
> while (--i >= 0) {
>