All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
Cc: Jiri Kosina <jkosina@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org, Ben Hutchings <ben@decadent.org.uk>
Subject: Re: [PATCH 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop
Date: Fri, 10 Aug 2012 13:11:12 -0400	[thread overview]
Message-ID: <20120810171112.GF5416@redhat.com> (raw)
In-Reply-To: <1344542391-16474-2-git-send-email-herton.krzesinski@canonical.com>

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

  reply	other threads:[~2012-08-10 17:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-09 19:59 Bug fixes/cleanups for floppy driver (v2) Herton Ronaldo Krzesinski
2012-08-09 19:59 ` [PATCH 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop Herton Ronaldo Krzesinski
2012-08-10 17:11   ` Vivek Goyal [this message]
2012-08-09 19:59 ` [PATCH 2/6] floppy: do put_disk on current dr if blk_init_queue fails Herton Ronaldo Krzesinski
2012-08-10 17:24   ` Vivek Goyal
2012-08-09 19:59 ` [PATCH 3/6] floppy: avoid leaking extra reference to queue on do_floppy_init error handling Herton Ronaldo Krzesinski
2012-08-10 17:25   ` Vivek Goyal
2012-08-09 19:59 ` [PATCH 4/6] floppy: properly handle failure on add_disk loop Herton Ronaldo Krzesinski
2012-08-10 17:34   ` Vivek Goyal
2012-08-09 19:59 ` [PATCH 5/6] floppy: remove check for allocated queue on do_floppy_init error handling Herton Ronaldo Krzesinski
2012-08-10 17:36   ` Vivek Goyal
2012-08-13 16:22     ` Herton Ronaldo Krzesinski
2012-08-09 19:59 ` [PATCH 6/6] floppy: use disk_registered for checking if a drive is present Herton Ronaldo Krzesinski
2012-08-10 17:35   ` Vivek Goyal
  -- strict thread matches above, loose matches on Subject: below --
2012-08-27 23:56 Bug fixes/cleanups for floppy driver (v4) Herton Ronaldo Krzesinski
2012-08-27 23:56 ` [PATCH 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop Herton Ronaldo Krzesinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120810171112.GF5416@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=ben@decadent.org.uk \
    --cc=herton.krzesinski@canonical.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.