Bug fixes/cleanups for floppy driver (v2)

Bug fixes/cleanups for floppy driver (v2)

[Herton Ronaldo Krzesinski]

Following this are fixes for bugs I noticed on floppy driver, and some
extra cleanups.

v2: separate fixes, and incorporate suggestions by Vivek Goyal.
    also I splitted the cleanups from fixes.

-- 
[]'s
Herton

[PATCH 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop

[Herton Ronaldo Krzesinski]

Since commit 070ad7e ("floppy: convert to delayed work and single-thread
wq"), we end up calling alloc_ordered_workqueue multiple times inside
the loop, which shouldn't be intended. Besides the leak, other side
effect in the current code is if blk_init_queue fails, we would end up
calling unregister_blkdev even if we didn't call yet register_blkdev.

Just moved the allocation of floppy_wq before the loop, and adjusted the
code accordingly.

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
Cc: stable@vger.kernel.org # 3.5+
---
 drivers/block/floppy.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index a7d6347..c8d9e68 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4138,6 +4138,10 @@ static int __init do_floppy_init(void)
 
 	raw_cmd = NULL;
 
+	floppy_wq = alloc_ordered_workqueue("floppy", 0);
+	if (!floppy_wq)
+		return -ENOMEM;
+
 	for (dr = 0; dr < N_DRIVE; dr++) {
 		disks[dr] = alloc_disk(1);
 		if (!disks[dr]) {
@@ -4145,16 +4149,10 @@ static int __init do_floppy_init(void)
 			goto out_put_disk;
 		}
 
-		floppy_wq = alloc_ordered_workqueue("floppy", 0);
-		if (!floppy_wq) {
-			err = -ENOMEM;
-			goto out_put_disk;
-		}
-
 		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
 		if (!disks[dr]->queue) {
 			err = -ENOMEM;
-			goto out_destroy_workq;
+			goto out_put_disk;
 		}
 
 		blk_queue_max_hw_sectors(disks[dr]->queue, 64);
@@ -4318,8 +4316,6 @@ out_release_dma:
 out_unreg_region:
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
 	platform_driver_unregister(&floppy_driver);
-out_destroy_workq:
-	destroy_workqueue(floppy_wq);
 out_unreg_blkdev:
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 out_put_disk:
@@ -4335,6 +4331,7 @@ out_put_disk:
 		}
 		put_disk(disks[dr]);
 	}
+	destroy_workqueue(floppy_wq);
 	return err;
 }
 
-- 
1.7.9.5

[PATCH 2/6] floppy: do put_disk on current dr if blk_init_queue fails

[Herton Ronaldo Krzesinski]

If blk_init_queue fails, we do not call put_disk on the current dr
(dr is decremented first in the error handling loop).

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
Cc: stable@vger.kernel.org
---
 drivers/block/floppy.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index c8d9e68..1e09e99 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4151,6 +4151,7 @@ static int __init do_floppy_init(void)
 
 		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
 		if (!disks[dr]->queue) {
+			put_disk(disks[dr]);
 			err = -ENOMEM;
 			goto out_put_disk;
 		}
-- 
1.7.9.5

[PATCH 3/6] floppy: avoid leaking extra reference to queue on do_floppy_init error handling

[Herton Ronaldo Krzesinski]

After commit 3f9a5aa ("floppy: Cleanup disk->queue before caling
put_disk() if add_disk() was never called"), if something fails in the
add_disk loop, we unconditionally set disks[dr]->queue to NULL. But
that's wrong, since we may have succesfully done an add_disk on some of
the drives previously in the loop, and in this case we would end up with
an extra reference to the disks[dr]->queue.

Add a new global array to mark "registered" disks, and use that to check
if we did an add_disk on one of the disks already. Using an array to
track added disks also will help to simplify/cleanup code later, as
suggested by Vivek Goyal.

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
Cc: stable@vger.kernel.org
---
 drivers/block/floppy.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 1e09e99..9272203 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -409,6 +409,7 @@ static struct floppy_drive_struct drive_state[N_DRIVE];
 static struct floppy_write_errors write_errors[N_DRIVE];
 static struct timer_list motor_off_timer[N_DRIVE];
 static struct gendisk *disks[N_DRIVE];
+static bool disk_registered[N_DRIVE];
 static struct block_device *opened_bdev[N_DRIVE];
 static DEFINE_MUTEX(open_lock);
 static struct floppy_raw_cmd *raw_cmd, default_raw_cmd;
@@ -4305,6 +4306,7 @@ static int __init do_floppy_init(void)
 		disks[drive]->flags |= GENHD_FL_REMOVABLE;
 		disks[drive]->driverfs_dev = &floppy_device[drive].dev;
 		add_disk(disks[drive]);
+		disk_registered[drive] = true;
 	}
 
 	return 0;
@@ -4328,7 +4330,8 @@ out_put_disk:
 			 * put_disk() is not paired with add_disk() and
 			 * will put queue reference one extra time. fix it.
 			 */
-			disks[dr]->queue = NULL;
+			if (!disk_registered[dr])
+				disks[dr]->queue = NULL;
 		}
 		put_disk(disks[dr]);
 	}
-- 
1.7.9.5

[PATCH 4/6] floppy: properly handle failure on add_disk loop

[Herton Ronaldo Krzesinski]

On do_floppy_init, if something failed inside the loop we call add_disk,
there was no cleanup of previous iterations in the error handling.

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
Cc: stable@vger.kernel.org
---
 drivers/block/floppy.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 9272203..3eafe93 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4294,7 +4294,7 @@ static int __init do_floppy_init(void)
 
 		err = platform_device_register(&floppy_device[drive]);
 		if (err)
-			goto out_release_dma;
+			goto out_remove_drives;
 
 		err = device_create_file(&floppy_device[drive].dev,
 					 &dev_attr_cmos);
@@ -4313,6 +4313,14 @@ static int __init do_floppy_init(void)
 
 out_unreg_platform_dev:
 	platform_device_unregister(&floppy_device[drive]);
+out_remove_drives:
+	while (drive--) {
+		if (disk_registered[drive]) {
+			del_gendisk(disks[drive]);
+			device_remove_file(&floppy_device[drive].dev, &dev_attr_cmos);
+			platform_device_unregister(&floppy_device[drive]);
+		}
+	}
 out_release_dma:
 	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();
-- 
1.7.9.5

[PATCH 5/6] floppy: remove check for allocated queue on do_floppy_init error handling

[Herton Ronaldo Krzesinski]

The check "if (disks[dr]->queue)" check is bogus, if we reach there
for each dr should exist an queue allocated (note that we decrement dr
first on entering the loop).

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 drivers/block/floppy.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3eafe93..438ffc9 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4332,15 +4332,13 @@ out_unreg_blkdev:
 out_put_disk:
 	while (dr--) {
 		del_timer_sync(&motor_off_timer[dr]);
-		if (disks[dr]->queue) {
-			blk_cleanup_queue(disks[dr]->queue);
-			/*
-			 * put_disk() is not paired with add_disk() and
-			 * will put queue reference one extra time. fix it.
-			 */
-			if (!disk_registered[dr])
-				disks[dr]->queue = NULL;
-		}
+		blk_cleanup_queue(disks[dr]->queue);
+		/*
+		 * put_disk() is not paired with add_disk() and
+		 * will put queue reference one extra time. fix it.
+		 */
+		if (!disk_registered[dr])
+			disks[dr]->queue = NULL;
 		put_disk(disks[dr]);
 	}
 	destroy_workqueue(floppy_wq);
-- 
1.7.9.5

[PATCH 6/6] floppy: use disk_registered for checking if a drive is present

[Herton Ronaldo Krzesinski]

Simplify/cleanup code, replacing remaining checks for drives present
using disk_registered array.

Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 drivers/block/floppy.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 438ffc9..5fcc2a1 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4114,9 +4114,7 @@ static struct platform_device floppy_device[N_DRIVE];
 static struct kobject *floppy_find(dev_t dev, int *part, void *data)
 {
 	int drive = (*part & 3) | ((*part & 0x80) >> 5);
-	if (drive >= N_DRIVE ||
-	    !(allowed_drive_mask & (1 << drive)) ||
-	    fdc_state[FDC(drive)].version == FDC_NONE)
+	if (drive >= N_DRIVE || !disk_registered[drive])
 		return NULL;
 	if (((*part >> 2) & 0x1f) >= ARRAY_SIZE(floppy_type))
 		return NULL;
@@ -4559,8 +4557,7 @@ static void __exit floppy_module_exit(void)
 	for (drive = 0; drive < N_DRIVE; drive++) {
 		del_timer_sync(&motor_off_timer[drive]);
 
-		if ((allowed_drive_mask & (1 << drive)) &&
-		    fdc_state[FDC(drive)].version != FDC_NONE) {
+		if (disk_registered[drive]) {
 			del_gendisk(disks[drive]);
 			device_remove_file(&floppy_device[drive].dev, &dev_attr_cmos);
 			platform_device_unregister(&floppy_device[drive]);
@@ -4571,8 +4568,7 @@ static void __exit floppy_module_exit(void)
 		 * These disks have not called add_disk().  Don't put down
 		 * queue reference in put_disk().
 		 */
-		if (!(allowed_drive_mask & (1 << drive)) ||
-		    fdc_state[FDC(drive)].version == FDC_NONE)
+		if (!disk_registered[drive])
 			disks[drive]->queue = NULL;
 
 		put_disk(disks[drive]);
-- 
1.7.9.5

Re: [PATCH 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:46PM -0300, Herton Ronaldo Krzesinski wrote:
> Since commit 070ad7e ("floppy: convert to delayed work and single-thread
> wq"), we end up calling alloc_ordered_workqueue multiple times inside
> the loop, which shouldn't be intended. Besides the leak, other side
> effect in the current code is if blk_init_queue fails, we would end up
> calling unregister_blkdev even if we didn't call yet register_blkdev.
> 
> Just moved the allocation of floppy_wq before the loop, and adjusted the
> code accordingly.
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> Cc: stable@vger.kernel.org # 3.5+

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

> ---
>  drivers/block/floppy.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index a7d6347..c8d9e68 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4138,6 +4138,10 @@ static int __init do_floppy_init(void)
>  
>  	raw_cmd = NULL;
>  
> +	floppy_wq = alloc_ordered_workqueue("floppy", 0);
> +	if (!floppy_wq)
> +		return -ENOMEM;
> +
>  	for (dr = 0; dr < N_DRIVE; dr++) {
>  		disks[dr] = alloc_disk(1);
>  		if (!disks[dr]) {
> @@ -4145,16 +4149,10 @@ static int __init do_floppy_init(void)
>  			goto out_put_disk;
>  		}
>  
> -		floppy_wq = alloc_ordered_workqueue("floppy", 0);
> -		if (!floppy_wq) {
> -			err = -ENOMEM;
> -			goto out_put_disk;
> -		}
> -
>  		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
>  		if (!disks[dr]->queue) {
>  			err = -ENOMEM;
> -			goto out_destroy_workq;
> +			goto out_put_disk;
>  		}
>  
>  		blk_queue_max_hw_sectors(disks[dr]->queue, 64);
> @@ -4318,8 +4316,6 @@ out_release_dma:
>  out_unreg_region:
>  	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
>  	platform_driver_unregister(&floppy_driver);
> -out_destroy_workq:
> -	destroy_workqueue(floppy_wq);
>  out_unreg_blkdev:
>  	unregister_blkdev(FLOPPY_MAJOR, "fd");
>  out_put_disk:
> @@ -4335,6 +4331,7 @@ out_put_disk:
>  		}
>  		put_disk(disks[dr]);
>  	}
> +	destroy_workqueue(floppy_wq);
>  	return err;
>  }
>  
> -- 
> 1.7.9.5

Re: [PATCH 2/6] floppy: do put_disk on current dr if blk_init_queue fails

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:47PM -0300, Herton Ronaldo Krzesinski wrote:
> If blk_init_queue fails, we do not call put_disk on the current dr
> (dr is decremented first in the error handling loop).
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> Cc: stable@vger.kernel.org

Hi,

So for the current drive we do put_disk() here and for rest of the drives
we do it in out_put_disk:.

How about if we go through all the N_DRIVE always and do put disk as need be.

	for(i = 0, i < N_DRIVE, i++) {
		if (!disks[i])
			continue;

		if (disks[i]->queue)
			blk_cleanup_queue();

		if (!disk_registered[i])
			disks[i]->queue = NULL;

		put_disk();
	}

It is little more lines of code but personally I find it easier to understand 
and less error prone as future modifications take place.

Thanks
Vivek

> ---
>  drivers/block/floppy.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index c8d9e68..1e09e99 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4151,6 +4151,7 @@ static int __init do_floppy_init(void)
>  
>  		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
>  		if (!disks[dr]->queue) {
> +			put_disk(disks[dr]);
>  			err = -ENOMEM;
>  			goto out_put_disk;
>  		}
> -- 
> 1.7.9.5

Re: [PATCH 3/6] floppy: avoid leaking extra reference to queue on do_floppy_init error handling

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:48PM -0300, Herton Ronaldo Krzesinski wrote:
> After commit 3f9a5aa ("floppy: Cleanup disk->queue before caling
> put_disk() if add_disk() was never called"), if something fails in the
> add_disk loop, we unconditionally set disks[dr]->queue to NULL. But
> that's wrong, since we may have succesfully done an add_disk on some of
> the drives previously in the loop, and in this case we would end up with
> an extra reference to the disks[dr]->queue.
> 
> Add a new global array to mark "registered" disks, and use that to check
> if we did an add_disk on one of the disks already. Using an array to
> track added disks also will help to simplify/cleanup code later, as
> suggested by Vivek Goyal.
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> Cc: stable@vger.kernel.org

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

> ---
>  drivers/block/floppy.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 1e09e99..9272203 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -409,6 +409,7 @@ static struct floppy_drive_struct drive_state[N_DRIVE];
>  static struct floppy_write_errors write_errors[N_DRIVE];
>  static struct timer_list motor_off_timer[N_DRIVE];
>  static struct gendisk *disks[N_DRIVE];
> +static bool disk_registered[N_DRIVE];
>  static struct block_device *opened_bdev[N_DRIVE];
>  static DEFINE_MUTEX(open_lock);
>  static struct floppy_raw_cmd *raw_cmd, default_raw_cmd;
> @@ -4305,6 +4306,7 @@ static int __init do_floppy_init(void)
>  		disks[drive]->flags |= GENHD_FL_REMOVABLE;
>  		disks[drive]->driverfs_dev = &floppy_device[drive].dev;
>  		add_disk(disks[drive]);
> +		disk_registered[drive] = true;
>  	}
>  
>  	return 0;
> @@ -4328,7 +4330,8 @@ out_put_disk:
>  			 * put_disk() is not paired with add_disk() and
>  			 * will put queue reference one extra time. fix it.
>  			 */
> -			disks[dr]->queue = NULL;
> +			if (!disk_registered[dr])
> +				disks[dr]->queue = NULL;
>  		}
>  		put_disk(disks[dr]);
>  	}
> -- 
> 1.7.9.5

Re: [PATCH 4/6] floppy: properly handle failure on add_disk loop

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:49PM -0300, Herton Ronaldo Krzesinski wrote:
> On do_floppy_init, if something failed inside the loop we call add_disk,
> there was no cleanup of previous iterations in the error handling.
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> Cc: stable@vger.kernel.org
> ---

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

>  drivers/block/floppy.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 9272203..3eafe93 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4294,7 +4294,7 @@ static int __init do_floppy_init(void)
>  
>  		err = platform_device_register(&floppy_device[drive]);
>  		if (err)
> -			goto out_release_dma;
> +			goto out_remove_drives;
>  
>  		err = device_create_file(&floppy_device[drive].dev,
>  					 &dev_attr_cmos);
> @@ -4313,6 +4313,14 @@ static int __init do_floppy_init(void)
>  
>  out_unreg_platform_dev:
>  	platform_device_unregister(&floppy_device[drive]);
> +out_remove_drives:
> +	while (drive--) {
> +		if (disk_registered[drive]) {
> +			del_gendisk(disks[drive]);
> +			device_remove_file(&floppy_device[drive].dev, &dev_attr_cmos);
> +			platform_device_unregister(&floppy_device[drive]);
> +		}
> +	}
>  out_release_dma:
>  	if (atomic_read(&usage_count))
>  		floppy_release_irq_and_dma();
> -- 
> 1.7.9.5

Re: [PATCH 6/6] floppy: use disk_registered for checking if a drive is present

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:51PM -0300, Herton Ronaldo Krzesinski wrote:
> Simplify/cleanup code, replacing remaining checks for drives present
> using disk_registered array.
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> ---

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

>  drivers/block/floppy.c |   10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 438ffc9..5fcc2a1 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4114,9 +4114,7 @@ static struct platform_device floppy_device[N_DRIVE];
>  static struct kobject *floppy_find(dev_t dev, int *part, void *data)
>  {
>  	int drive = (*part & 3) | ((*part & 0x80) >> 5);
> -	if (drive >= N_DRIVE ||
> -	    !(allowed_drive_mask & (1 << drive)) ||
> -	    fdc_state[FDC(drive)].version == FDC_NONE)
> +	if (drive >= N_DRIVE || !disk_registered[drive])
>  		return NULL;
>  	if (((*part >> 2) & 0x1f) >= ARRAY_SIZE(floppy_type))
>  		return NULL;
> @@ -4559,8 +4557,7 @@ static void __exit floppy_module_exit(void)
>  	for (drive = 0; drive < N_DRIVE; drive++) {
>  		del_timer_sync(&motor_off_timer[drive]);
>  
> -		if ((allowed_drive_mask & (1 << drive)) &&
> -		    fdc_state[FDC(drive)].version != FDC_NONE) {
> +		if (disk_registered[drive]) {
>  			del_gendisk(disks[drive]);
>  			device_remove_file(&floppy_device[drive].dev, &dev_attr_cmos);
>  			platform_device_unregister(&floppy_device[drive]);
> @@ -4571,8 +4568,7 @@ static void __exit floppy_module_exit(void)
>  		 * These disks have not called add_disk().  Don't put down
>  		 * queue reference in put_disk().
>  		 */
> -		if (!(allowed_drive_mask & (1 << drive)) ||
> -		    fdc_state[FDC(drive)].version == FDC_NONE)
> +		if (!disk_registered[drive])
>  			disks[drive]->queue = NULL;
>  
>  		put_disk(disks[drive]);
> -- 
> 1.7.9.5

Re: [PATCH 5/6] floppy: remove check for allocated queue on do_floppy_init error handling

[Vivek Goyal]

On Thu, Aug 09, 2012 at 04:59:50PM -0300, Herton Ronaldo Krzesinski wrote:
> The check "if (disks[dr]->queue)" check is bogus, if we reach there
> for each dr should exist an queue allocated (note that we decrement dr
> first on entering the loop).
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>

As mentioned in second patch, I like going trhough full array of drives
and do cleanup as needed instead of relying on "dr" variable. 

But if you don't like that, then I am not as such against this approach.
Was just trying to keep all put_disk() at one place.

Thanks
Vivek

> ---
>  drivers/block/floppy.c |   16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 3eafe93..438ffc9 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4332,15 +4332,13 @@ out_unreg_blkdev:
>  out_put_disk:
>  	while (dr--) {
>  		del_timer_sync(&motor_off_timer[dr]);
> -		if (disks[dr]->queue) {
> -			blk_cleanup_queue(disks[dr]->queue);
> -			/*
> -			 * put_disk() is not paired with add_disk() and
> -			 * will put queue reference one extra time. fix it.
> -			 */
> -			if (!disk_registered[dr])
> -				disks[dr]->queue = NULL;
> -		}
> +		blk_cleanup_queue(disks[dr]->queue);
> +		/*
> +		 * put_disk() is not paired with add_disk() and
> +		 * will put queue reference one extra time. fix it.
> +		 */
> +		if (!disk_registered[dr])
> +			disks[dr]->queue = NULL;
>  		put_disk(disks[dr]);
>  	}
>  	destroy_workqueue(floppy_wq);
> -- 
> 1.7.9.5

Re: [PATCH 5/6] floppy: remove check for allocated queue on do_floppy_init error handling

[Herton Ronaldo Krzesinski]

On Fri, Aug 10, 2012 at 01:36:53PM -0400, Vivek Goyal wrote:
> On Thu, Aug 09, 2012 at 04:59:50PM -0300, Herton Ronaldo Krzesinski wrote:
> > The check "if (disks[dr]->queue)" check is bogus, if we reach there
> > for each dr should exist an queue allocated (note that we decrement dr
> > first on entering the loop).
> > 
> > Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
> 
> As mentioned in second patch, I like going trhough full array of drives
> and do cleanup as needed instead of relying on "dr" variable. 
> 
> But if you don't like that, then I am not as such against this approach.
> Was just trying to keep all put_disk() at one place.

Ok, I'll drop this, and do a full cleanup. I just ended up doing the
minimal work, but I'm not against cleaning this. I still want to keep
the second patch, as being a simple/appropriate bug fix for stable. I'll
submit a new series soon. Thanks for looking at this series.

> 
> Thanks
> Vivek
> 

-- 
[]'s
Herton

[PATCH 2/6] floppy: do put_disk on current dr if blk_init_queue fails

[Herton Ronaldo Krzesinski]

If blk_init_queue fails, we do not call put_disk on the current dr
(dr is decremented first in the error handling loop).

Cc: stable@vger.kernel.org
Reviewed-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 drivers/block/floppy.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index c8d9e68..1e09e99 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4151,6 +4151,7 @@ static int __init do_floppy_init(void)
 
 		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
 		if (!disks[dr]->queue) {
+			put_disk(disks[dr]);
 			err = -ENOMEM;
 			goto out_put_disk;
 		}
-- 
1.7.9.5