All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>,
	Jens Axboe <axboe@kernel.dk>, Jiri Kosina <jkosina@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org, Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH v3 3/6] floppy: avoid leaking extra reference to queue on do_floppy_init error handling
Date: Tue, 14 Aug 2012 11:03:30 +0200	[thread overview]
Message-ID: <20120814090330.GC2423@redhat.com> (raw)
In-Reply-To: <1344914439.824.178.camel@deadeye.wl.decadent.org.uk>

On Tue, Aug 14, 2012 at 04:20:39AM +0100, Ben Hutchings wrote:
> On Mon, 2012-08-13 at 15:16 -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.
> [...]
> 
> It's totally ridiculous that a driver should have to do this.  Any
> registered disk should have the GENHD_FL_UP flag set... so why can't
> genhd check it?  It doesn't look like floppy is the only driver affected
> by this problem, either.  So I suggest the following general fix
> (untested):
> 
> ---
> Subject: genhd: Make put_disk() safe for disks that have not been registered
> 
> Since commit 9f53d2f ('block: fix __blkdev_get and add_disk race
> condition'), add_disk() adds a reference to disk->queue,

I do not see this? Commit 9f53d2fe insert disk_alloc_events() to add_disk(),
but disk_alloc_events() function does not get any reference to disk->queue,
I missed something?

Stanislaw

  reply	other threads:[~2012-08-14  9:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-13 18:16 Bug fixes/cleanups for floppy driver (v2) Herton Ronaldo Krzesinski
2012-08-13 18:16 ` [PATCH v3 1/6] floppy: don't call alloc_ordered_workqueue inside the alloc_disk loop Herton Ronaldo Krzesinski
2012-08-14  2:01   ` Ben Hutchings
2012-08-13 18:16 ` [PATCH v3 2/6] floppy: do put_disk on current dr if blk_init_queue fails Herton Ronaldo Krzesinski
2012-08-14  2:04   ` Ben Hutchings
2012-08-14 19:48   ` Vivek Goyal
2012-08-14 20:08     ` Herton Ronaldo Krzesinski
2012-08-13 18:16 ` [PATCH v3 3/6] floppy: avoid leaking extra reference to queue on do_floppy_init error handling Herton Ronaldo Krzesinski
2012-08-14  3:20   ` Ben Hutchings
2012-08-14  9:03     ` Stanislaw Gruszka [this message]
2012-08-14 14:26       ` Herton Ronaldo Krzesinski
2012-08-14 14:28       ` Ben Hutchings
2012-08-14 14:33     ` Herton Ronaldo Krzesinski
2012-08-14 20:00     ` Vivek Goyal
2012-08-13 18:16 ` [PATCH v3 4/6] floppy: properly handle failure on add_disk loop Herton Ronaldo Krzesinski
2012-08-14  3:31   ` Ben Hutchings
2012-08-14 14:43     ` Herton Ronaldo Krzesinski
2012-08-27 23:53       ` Herton Ronaldo Krzesinski
2012-08-13 18:16 ` [PATCH v3 5/6] floppy: use disk_registered for checking if a drive is present Herton Ronaldo Krzesinski
2012-08-14  3:36   ` Ben Hutchings
2012-08-13 18:16 ` [PATCH v3 6/6] floppy: remove dr, reuse drive on do_floppy_init Herton Ronaldo Krzesinski
2012-08-13 18:21 ` Bug fixes/cleanups for floppy driver (v2) 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=20120814090330.GC2423@redhat.com \
    --to=sgruszka@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 \
    --cc=vgoyal@redhat.com \
    /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.