All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@dreamhost.com>
To: ceph-devel@vger.kernel.org
Subject: [PATCH] rbd: don't drop the rbd_id too early
Date: Tue, 28 Feb 2012 20:42:51 -0800	[thread overview]
Message-ID: <4F4DACCB.8000404@dreamhost.com> (raw)

Currently an rbd device's id is released when it is removed, but it
is done before the code is run to clean up sysfs-related files (such
as /sys/bus/rbd/devices/1).

It's possible that an rbd is still in use after the rbd_remove()
call has been made.  It's essentially the same as an active inode
that stays around after it has been removed--until its final close
operation.  This means that the id shows up as free for reuse at a
time it should not be.

The effect of this was seen by Jens Rehpoehler, who:
     - had a filesystem mounted on an rbd device
     - unmapped that filesystem (without unmounting)
     - found that the mount still worked properly
     - but hit a panic when he attempted to re-map a new rbd device

This re-map attempt found the previously-unmapped id available.
The subsequent attempt to reuse it was met with a panic while
attempting to (re-)install the sysfs entry for the new mapped
device.

Fix this by holding off "putting" the rbd id, until the rbd_device
release function is called--when the last reference is finally
dropped.

Note: This fixes: http://tracker.newdream.net/issues/1907

Signed-off-by: Alex Elder <elder@dreamhost.com>
---
  drivers/block/rbd.c |   14 +++++++++-----
  1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index da38804..7cae4a3 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2440,7 +2440,12 @@ static ssize_t rbd_add(struct bus_type *bus,
  	if (rc)
  		goto err_out_blkdev;

-	/* set up and announce blkdev mapping */
+	/*
+	 * At this point cleanup in the event of an error is the job
+	 * of the sysfs code (initiated by rbd_bus_del_dev()).
+	 *
+	 * Set up and announce blkdev mapping.
+	 */
  	rc = rbd_init_disk(rbd_dev);
  	if (rc)
  		goto err_out_bus;
@@ -2452,8 +2457,6 @@ static ssize_t rbd_add(struct bus_type *bus,
  	return count;

  err_out_bus:
-	rbd_id_put(rbd_dev);
-
  	/* this will also clean up rest of rbd_dev stuff */

  	rbd_bus_del_dev(rbd_dev);
@@ -2511,6 +2514,9 @@ static void rbd_dev_release(struct device *dev)
  	/* clean up and free blkdev */
  	rbd_free_disk(rbd_dev);
  	unregister_blkdev(rbd_dev->major, rbd_dev->name);
+
+	/* done with the id, and with the rbd_dev */
+	rbd_id_put(rbd_dev);
  	kfree(rbd_dev);

  	/* release module ref */
@@ -2543,8 +2549,6 @@ static ssize_t rbd_remove(struct bus_type *bus,
  		goto done;
  	}

-	rbd_id_put(rbd_dev);
-
  	__rbd_remove_all_snaps(rbd_dev);
  	rbd_bus_del_dev(rbd_dev);

-- 
1.7.5.4


             reply	other threads:[~2012-02-29  4:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-29  4:42 Alex Elder [this message]
2012-03-02 21:33 ` [PATCH] rbd: don't drop the rbd_id too early Sage Weil

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=4F4DACCB.8000404@dreamhost.com \
    --to=elder@dreamhost.com \
    --cc=ceph-devel@vger.kernel.org \
    /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.