[PATCH 1/2] Disk hot removal causing oopses and fixes

I am proposing two patches to protect the request queue and io
elevator from inadvertent releasing.

I've been testing mmc driver robustness against rapid card removal
reinsert cycles and it has been rather easy to get a kernel oopses
(on 2.6.2 because of insufficient locking.

When MMC driver notices a card has been removed, it removes the
card and the block device. The call proceeds to
blk_cleanup_queue() which marks the request queue dead, calls
elevator exit to release the elevator and puts request queue.
This releases elevator always and may also release the request
queue.

If file system is still mounted and using the disk after
blk_cleanup_queue() has been called, io requests will access
already free'ed structures and oopses and BUG_ON()s jump in.

When the proposed patches are applied, I am not able reproduce
the oopses in io scheduler or elevator anymore. There is still
another oops is sysfs truing to add duplicate file, but i think
that is not related.

Cheers
Jarkko Lavinen

From 9559377f3166345649c3406427f410cd51472944 Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen DeleteThis @nokia.com>
Date: Wed, 21 Oct 2009 18:48:18 +0300
Subject: [PATCH 1/2] block: Avoid dead request queue too early removal

If disk is removed while file system is still mounted, the disk
removal can release the dead request queue too early while
file system is still trying to sumit requests.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen DeleteThis @nokia.com>
---
fs/block_dev.c | 24 ++++++++++++++++++++----
1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9cf4b92..91f2fc3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1165,6 +1165,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
int ret;
int partno;
int perm = 0;
+ int q_ref = 0;
+ struct module *owner;

if (mode & FMODE_READ)
perm |= MAY_READ;
@@ -1187,6 +1189,11 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
if (!disk)
goto out_unlock_kernel;

+ if (blk_get_queue(disk->queue))
+ goto out_unlock_kernel;
+ else
+ q_ref = 1;
+
mutex_lock_nested(&bdev->bd_mutex, for_part);
if (!bdev->bd_openers) {
bdev->bd_disk = disk;
@@ -1248,8 +1255,10 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
}
} else {
+ owner = disk->fops->owner;
+ blk_put_queue(disk->queue);
put_disk(disk);
- module_put(disk->fops->owner);
+ module_put(owner);
disk = NULL;
if (bdev->bd_contains == bdev) {
if (bdev->bd_disk->fops->open) {
@@ -1281,9 +1290,15 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
out_unlock_kernel:
unlock_kernel();

- if (disk)
- module_put(disk->fops->owner);
- put_disk(disk);
+ if (disk) {
+ owner = disk->fops->owner;
+ if (q_ref)
+ blk_put_queue(disk->queue);
+
+ put_disk(disk);
+ module_put(owner);
+ }
+
bdput(bdev);

return ret;
@@ -1360,6 +1375,7 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
if (!bdev->bd_openers) {
struct module *owner = disk->fops->owner;

+ blk_put_queue(disk->queue);
put_disk(disk);
module_put(owner);
disk_put_part(bdev->bd_part);
--
1.6.3.3

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

From ff822af94f86dbe559b7649b7f71c464260ef353 Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen.TakeThisOut@nokia.com>
Date: Wed, 21 Oct 2009 18:53:20 +0300
Subject: [PATCH 2/2] block: Protect elevator from too early release

When a disk is being removed, blk_put_queue calls elevator exit function
which will release the elevator while the elevator may still be in use.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen.TakeThisOut@nokia.com>
---
block/blk-core.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index ac0fa10..da56586 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1173,6 +1173,13 @@ static int __make_request(struct request_queue *q, struct bio *bio)
*/
blk_queue_bounce(q, &bio);

+ mutex_lock(&q->elevator->sysfs_lock);
+ if (test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)) {
+ mutex_unlock(&q->elevator->sysfs_lock);
+ bio_endio(bio, -EIO);
+ return 0;
+ }
+
spin_lock_irq(q->queue_lock);

if (unlikely(bio_rw_flagged(bio, BIO_RW_BARRIER)) || elv_queue_empty(q))
@@ -1275,6 +1282,8 @@ out:
if (unplug || !queue_should_plug(q))
__generic_unplug_device(q);
spin_unlock_irq(q->queue_lock);
+ mutex_unlock(&q->elevator->sysfs_lock);
+
return 0;
}

--
1.6.3.3

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