linux-kernel - Re: How to cleanly shut down a block device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Nov 2006 12:33:47 +0100
From:	Pierre Ossman <drzeus-list@...eus.cx>
To:	Jens Axboe <jens.axboe@...cle.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: How to cleanly shut down a block device

Russell King wrote:
> Just arrange for the mmc_queue_thread() to empty the queue when
> MMC_QUEUE_EXIT is set, and then exit.  I thought this was something
> that the block layer looked after (Jens must have missed this in his
> original review of the MMC code.)
>   

mmc_queue_thread() will empty the thread when MMC_QUEUE_EXIT is set. The
problem is that we do not set that bit until the last person closes the
device. In order to avoid problems we need to empty the queue before
mmc_blk_remove() exits (after which the card structure is no longer valid).

> The handling of userspace keeping the device open despite the hardware
> having been removed is already in place.
>
>   

Ok, that's one less problem for me to worry about. :)

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);

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/