All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mikulas Patocka <mpatocka@redhat.com>
To: Alasdair G Kergon <agk@redhat.com>
Cc: dm-devel@redhat.com
Subject: [PATCH 2/2] dm-bdev-keep-bdev-always-referenced.patch
Date: Tue, 28 Apr 2009 12:14:31 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0904281214120.22533@hs20-bc2-1.build.redhat.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0904281211590.22533@hs20-bc2-1.build.redhat.com>

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.

It caused bug https://bugzilla.redhat.com/show_bug.cgi?id=179786

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 |   37 +++++++++----------------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

Index: linux-2.6.30-rc2-devel/drivers/md/dm.c
===================================================================
--- linux-2.6.30-rc2-devel.orig/drivers/md/dm.c	2009-04-15 16:24:19.000000000 +0200
+++ linux-2.6.30-rc2-devel/drivers/md/dm.c	2009-04-15 16:24:25.000000000 +0200
@@ -1183,6 +1183,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);
@@ -1192,6 +1196,8 @@ static struct mapped_device *alloc_dev(i
 
 	return md;
 
+bad_bdev:
+	destroy_workqueue(md->wq);
 bad_thread:
 	put_disk(md->disk);
 bad_disk:
@@ -1217,10 +1223,8 @@ static void free_dev(struct mapped_devic
 {
 	int minor = MINOR(disk_devt(md->disk));
 
-	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);
@@ -1280,8 +1284,7 @@ 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) {
 		dm_table_destroy(t);
@@ -1523,11 +1526,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);
 
@@ -1606,13 +1604,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 out;
-		}
-
 		/*
 		 * Flush I/O to the device. noflush supersedes do_lockfs,
 		 * because lock_fs() needs to flush I/Os.
@@ -1678,11 +1669,6 @@ int dm_suspend(struct mapped_device *md,
 	set_bit(DMF_SUSPENDED, &md->flags);
 
 out:
-	if (r && md->bdev) {
-		bdput(md->bdev);
-		md->bdev = NULL;
-	}
-
 	dm_table_put(map);
 
 out_unlock:
@@ -1711,11 +1697,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);

  reply	other threads:[~2009-04-28 16:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-28 16:14 [PATCH 1/2] dm-bdev-rename-suspended_bdev-to-bdev.patch Mikulas Patocka
2009-04-28 16:14 ` Mikulas Patocka [this message]
2009-04-29  6:26   ` [PATCH 2/2] dm-bdev-keep-bdev-always-referenced.patch Mikulas Patocka
2009-05-12  8:32     ` Jun'ichi Nomura
2009-05-18 15:34       ` Mikulas Patocka
2009-05-19  1:36         ` Jun'ichi Nomura
2009-05-18 15:35       ` [PATCH] use i_size_read() instead of i_size Mikulas Patocka
2009-07-23  8:52       ` [PATCH 2/2] dm-bdev-keep-bdev-always-referenced.patch Jun'ichi Nomura
2009-07-27 23:44         ` [PATCH] drop mutex in __set_size Mikulas Patocka
2009-07-29  0:14           ` Jun'ichi Nomura
2009-07-30 15:17             ` Mikulas Patocka
2009-07-30 23:49               ` Jun'ichi Nomura

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=Pine.LNX.4.64.0904281214120.22533@hs20-bc2-1.build.redhat.com \
    --to=mpatocka@redhat.com \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    /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.