Attachment 309021 Details for Bug 179786 – fix the deadlock (in upstream code)

[patch] fix the deadlock (in upstream code)

dm-bdev-keep-bdev-always-referenced.patch (text/plain), 3.42 KB, created by Mikuláš Patočka on 2008-06-12 01:51:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mikuláš Patočka

Created: 2008-06-12 01:51:26 UTC

Size: 3.42 KB

patch

obsolete

>Always keep the reference to structure block_device.
>
>Reference to block_device is obtained with function bdget_disk (and is released
>with bdput.
>
>bdget_disk was called while the device was being suspended, in dm_suspend() ---
>however at this point, there could be another devices already suspended.
>
>bdget_disk can wait for IO and allocate memory and this could result in waiting
>for already suspended device, resulting in deadlock.
>
>This patch changes the code so that it gets the reference to struct block_device
>when struct mapped_device is allocated and initialized in (alloc_dev) and drops
>the reference when it is destroyed in free_dev. Thus, there is no call to
>bdget_disk, while any device is suspended.
>
>Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
>---
> drivers/md/dm.c |   46 ++++++++++------------------------------------
> 1 file changed, 10 insertions(+), 36 deletions(-)
>
>Index: linux-2.6.26-rc4/drivers/md/dm.c
>===================================================================
>--- linux-2.6.26-rc4.orig/drivers/md/dm.c	2008-06-11 19:22:09.000000000 +0200
>+++ linux-2.6.26-rc4/drivers/md/dm.c	2008-06-11 19:22:14.000000000 +0200
>@@ -1066,6 +1066,10 @@ static struct mapped_device *alloc_dev(i
> 	if (!md->wq)
> 		goto bad_thread;
> 
>+	md->bdev = bdget_disk(md->disk, 0);
>+	if (!md->bdev)
>+		goto bad_bdev;
>+
> 	/* Populate the mapping, nobody knows we exist yet */
> 	spin_lock(&_minor_lock);
> 	old_md = idr_replace(&_minor_idr, md, minor);
>@@ -1075,6 +1079,8 @@ static struct mapped_device *alloc_dev(i
> 
> 	return md;
> 
>+bad_bdev:
>+	destroy_workqueue(md->wq);
> bad_thread:
> 	put_disk(md->disk);
> bad_disk:
>@@ -1100,10 +1106,8 @@ static void free_dev(struct mapped_devic
> {
> 	int minor = md->disk->first_minor;
> 
>-	if (md->bdev) {
>-		unlock_fs(md);
>-		bdput(md->bdev);
>-	}
>+	unlock_fs(md);
>+	bdput(md->bdev);
> 	destroy_workqueue(md->wq);
> 	mempool_destroy(md->tio_pool);
> 	mempool_destroy(md->io_pool);
>@@ -1162,8 +1166,8 @@ static int __bind(struct mapped_device *
> 	if (size != get_capacity(md->disk))
> 		memset(&md->geometry, 0, sizeof(md->geometry));
> 
>-	if (md->bdev)
>-		__set_size(md, size);
>+	__set_size(md, size);
>+
> 	if (size == 0)
> 		return 0;
> 
>@@ -1384,11 +1388,6 @@ int dm_swap_table(struct mapped_device *
> 	if (!dm_suspended(md))
> 		goto out;
> 
>-	/* without bdev, the device size cannot be changed */
>-	if (!md->bdev)
>-		if (get_capacity(md->disk) != dm_table_get_size(table))
>-			goto out;
>-
> 	__unbind(md);
> 	r = __bind(md, table);
> 
>@@ -1468,13 +1467,6 @@ int dm_suspend(struct mapped_device *md,
> 
> 	/* bdget() can stall if the pending I/Os are not flushed */
> 	if (!noflush) {
>-		md->bdev = bdget_disk(md->disk, 0);
>-		if (!md->bdev) {
>-			DMWARN("bdget failed in dm_suspend");
>-			r = -ENOMEM;
>-			goto flush_and_out;
>-		}
>-
> 		/*
> 		 * Flush I/O to the device. noflush supersedes do_lockfs,
> 		 * because lock_fs() needs to flush I/Os.
>@@ -1523,20 +1515,7 @@ int dm_suspend(struct mapped_device *md,
> 
> 	set_bit(DMF_SUSPENDED, &md->flags);
> 
>-flush_and_out:
>-	if (r && noflush)
>-		/*
>-		 * Because there may be already I/Os in the pushback list,
>-		 * flush them before return.
>-		 */
>-		dm_queue_flush(md, DM_WQ_FLUSH_ALL, NULL);
>-
> out:
>-	if (r && md->bdev) {
>-		bdput(md->bdev);
>-		md->bdev = NULL;
>-	}
>-
> 	dm_table_put(map);
> 
> out_unlock:
>@@ -1565,11 +1544,6 @@ int dm_resume(struct mapped_device *md)
> 
> 	unlock_fs(md);
> 
>-	if (md->bdev) {
>-		bdput(md->bdev);
>-		md->bdev = NULL;
>-	}
>-
> 	clear_bit(DMF_SUSPENDED, &md->flags);
> 
> 	dm_table_unplug_all(map);

Actions: View | Diff

Attachments on bug 179786: 309020 | 309021