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

[Michael Schmitz <schmitzmic@gmail.com>]

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) {
>