From: Russell King <rmk+lkml@arm.linux.org.uk>
To: Pierre Ossman <drzeus-list@drzeus.cx>
Cc: Jens Axboe <jens.axboe@oracle.com>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: How to cleanly shut down a block device
Date: Tue, 14 Nov 2006 11:48:57 +0000 [thread overview]
Message-ID: <20061114114857.GC15340@flint.arm.linux.org.uk> (raw)
In-Reply-To: <4559A99B.6070207@drzeus.cx>
On Tue, Nov 14, 2006 at 12:33:47PM +0100, Pierre Ossman wrote:
> Jens Axboe wrote:
> > What do you mean by "killing off the queue"? As long as the queue can be
> > gotten at, it needs to remain valid. That is what the references are
> > for.
> >
>
> I do:
>
> del_gendisk();
> (wait for queue to become empty, i.e. elv_next_request() == NULL)
> blk_cleanup_queue();
>
> and then assume that the request function will no longer be called for
> this queue.
>
> Suggested patch:
>
> diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
> index f9027c8..5025abe 100644
> --- a/drivers/mmc/mmc_block.c
> +++ b/drivers/mmc/mmc_block.c
> @@ -83,7 +83,6 @@ static void mmc_blk_put(struct mmc_blk_d
> md->usage--;
> if (md->usage == 0) {
> put_disk(md->disk);
> - mmc_cleanup_queue(&md->queue);
> kfree(md);
> }
> mutex_unlock(&open_lock);
> @@ -553,12 +552,11 @@ static void mmc_blk_remove(struct mmc_ca
> if (md) {
> int devidx;
>
> + /* Stop new requests from getting into the queue */
> del_gendisk(md->disk);
>
> - /*
> - * I think this is needed.
> - */
> - md->disk->queue = NULL;
> + /* Then flush out any already in there */
> + mmc_cleanup_queue(&md->queue);
>
> devidx = md->disk->first_minor >> MMC_SHIFT;
> __clear_bit(devidx, dev_use);
You now have a disk which may be in use (and a block device) for which
there is no queue. I couldn't see any locking what so ever in del_gendisk
so it's quite possible that the queue could still be referenced while
the disk is still open.
I also don't think del_gendisk() is sufficient to ensure that no new
requests appear in the queue, which is why I'm setting md->disk->queue
to NULL there.
But shug, I don't know the block layer. Jens is your best bet.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
prev parent reply other threads:[~2006-11-14 11:49 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-14 7:02 How to cleanly shut down a block device Pierre Ossman
2006-11-14 7:56 ` Jens Axboe
2006-11-14 8:15 ` Pierre Ossman
2006-11-14 8:45 ` Jens Axboe
2006-11-14 8:54 ` Pierre Ossman
2006-11-14 10:24 ` Jens Axboe
2006-11-14 10:48 ` Russell King
2006-11-14 11:33 ` Pierre Ossman
2006-11-14 11:41 ` Jens Axboe
2006-11-14 11:52 ` Pierre Ossman
2006-11-14 14:34 ` Pierre Ossman
2006-11-14 20:48 ` Pierre Ossman
2006-11-23 21:03 ` Russell King
2006-11-23 21:19 ` Pierre Ossman
2006-11-14 11:48 ` Russell King [this message]
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=20061114114857.GC15340@flint.arm.linux.org.uk \
--to=rmk+lkml@arm.linux.org.uk \
--cc=drzeus-list@drzeus.cx \
--cc=jens.axboe@oracle.com \
--cc=linux-kernel@vger.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.