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
next prev parent 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.