On Tue, Nov 14, 2006 at 03:34:25PM +0100, Pierre Ossman wrote:
> How about this patch? It is basically the same as the previous, but it
> also sets queuedata to NULL and tests for that. It does not address if
> someone still has dependencies on the queue but hasn't gotten itself a
> reference (as I haven't gotten any word on if that is a problem):
>
> 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'm not entirely convinced that del_gendisk() will prevent any new
requests getting into the queue, which is why the comment below
says:
>
> - /*
> - * I think this is needed.
> - */
> - md->disk->queue = NULL;
Yes, del_gendisk() removes the disk from view for new opens, but what
about existing users?
As the code currently stands, when I remove a mounted MMC card, I get:
generic_make_request: Trying to access nonexistent block-device mmcblk0p1 (26)
FAT: Directory bread(block 26) failed
generic_make_request: Trying to access nonexistent block-device mmcblk0p1 (27)
FAT: Directory bread(block 27) failed
... etc ...
and this comes from:
q = bdev_get_queue(bio->bi_bdev);
if (!q) {
printk(KERN_ERR ...
in block/ll_rw_blk.c. bdev_get_queue merely does:
return bdev->bd_disk->queue;
> + /* Then flush out any already in there */
> + mmc_cleanup_queue(&md->queue);
Now, the thing is calling mmc_cleanup_queue() will shutdown the thread
and wait for it to complete its business, free the scatterlist, and
call blk_cleanup_queue() on that queue.
Ergo, once we've shut down the thread, we should not get any further
MMC requests, so:
> diff --git a/drivers/mmc/mmc_queue.c b/drivers/mmc/mmc_queue.c
> index 4ccdd82..b6769e2 100644
> --- a/drivers/mmc/mmc_queue.c
> +++ b/drivers/mmc/mmc_queue.c
> @@ -111,6 +111,19 @@ static int mmc_queue_thread(void *d)
> static void mmc_request(request_queue_t *q)
> {
> struct mmc_queue *mq = q->queuedata;
> + struct request *req;
> + int ret;
> +
> + if (!mq) {
> + printk(KERN_ERR "MMC: killing requests for dead queue\n");
> + while ((req = elv_next_request(q)) != NULL) {
> + do {
> + ret = end_that_request_chunk(req, 0,
> + req->current_nr_sectors << 9);
> + } while (ret);
> + }
> + return;
> + }
>
> if (!mq->req)
> wake_up(&mq->thread_wq);
> @@ -179,6 +192,15 @@ EXPORT_SYMBOL(mmc_init_queue);
>
> void mmc_cleanup_queue(struct mmc_queue *mq)
> {
> + request_queue_t *q = mq->queue;
> + unsigned long flags;
> +
> + /* Mark that we should start throwing out stragglers */
> + spin_lock_irqsave(q->queue_lock, flags);
> + q->queuedata = NULL;
> + spin_unlock_irqrestore(q->queue_lock, flags);
> +
> + /* Then terminate our worker thread */
> mq->flags |= MMC_QUEUE_EXIT;
> wake_up(&mq->thread_wq);
> wait_for_completion(&mq->thread_complete);
These hunks do not achieve us anything.
However, what we _do_ need to do is to arrange for the MMC queue thread
to error out all pending requests before dying if MMC_QUEUE_EXIT is set.
That's already handled since the queue thread only ever exits if there
are no requests pending _and_ MMC_QUEUE_EXIT has been set.
So I think all you really need is this change:
diff --git a/drivers/mmc/mmc_block.c b/drivers/mmc/mmc_block.c
index f9027c8..9173067 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);
@@ -559,6 +558,7 @@ static void mmc_blk_remove(struct mmc_ca
* I think this is needed.
*/
md->disk->queue = NULL;
+ mmc_cleanup_queue(&md->queue);
devidx = md->disk->first_minor >> MMC_SHIFT;
__clear_bit(devidx, dev_use);
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
