xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0 of 8] Linux/pvops blktap + blkfront updates.
@ 2010-02-26 16:04 Daniel Stodden
  2010-02-26 16:04 ` [PATCH 1 of 8] blktap: Fix tap kobj double unref in sysfs_destroy Daniel Stodden
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge


First 4 stop blktap barfing once and blkfront twice during shutdown,
plus a memory leak.

16 A fix-double-free.diff: blktap: Fix tap kobj double unref in sysfs_destroy.
17 A fix-blk-cleanup.diff: blkfront: Fix backtrace in blk_cleanup_queue.
18 A fix-gendisk-leak.diff: blkfront: Stop leaking the gendisk struct.
19 A fix-bd-oops.diff: blkfront: Fix an unlikely xenbus crasher.

The rest is decoration.

20 A blkfront-close.diff: blkfront: Clean up closing transition.
21 A xs-write-unlocked.diff: blkfront: Move xenstore accesses off the device release path.
22 A xlvbd-release-gendisk.diff: blkfront: Clean up blkfront_closing.
23 A debug-common.diff: blkfront: Clean up debug statements.

Cheers,
Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1 of 8] blktap: Fix tap kobj double unref in sysfs_destroy
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 2 of 8] blkfront: Fix backtrace in blk_cleanup_queue Daniel Stodden
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

It's that final put_device in device_unregister which is supposed to
clear the caller's reference. Subtle indeed, because refcount went
2->3 when after sysfs_destroy:

 +1 for our own ring->dev
 +1 for the open sysfs ./remove attribute
 +1 for the callback scheduled

Drops to 1 in device_unregister and 0 only after leaving callbacks.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: fix-double-free.diff --]
[-- Type: text/x-patch, Size: 1014 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267197951 28800
# Node ID 3ea95de27eaed11b87a5c608629f5089b0b33a73
# Parent  6cee7e3206311d4556e5bb75ceb522e4e293d4d1
blktap: Fix tap kobj double unref in sysfs_destroy.

It's that final put_device in device_unregister which is supposed to
clear the caller's reference. Subtle indeed, because refcount went
2->3 when after sysfs_destroy:

 +1 for our own ring->dev
 +1 for the open sysfs ./remove attribute
 +1 for the callback scheduled

Drops to 1 in device_unregister and 0 only after leaving callbacks.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 6cee7e320631 -r 3ea95de27eae drivers/xen/blktap/sysfs.c
--- a/drivers/xen/blktap/sysfs.c	Fri Feb 26 07:25:50 2010 -0800
+++ b/drivers/xen/blktap/sysfs.c	Fri Feb 26 07:25:51 2010 -0800
@@ -360,7 +360,6 @@
 				     !atomic_read(&tap->ring.sysfs_refcnt)))
 		return -EAGAIN;
 
-	put_device(dev);
 	device_schedule_callback(dev, device_unregister);
 
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2 of 8] blkfront: Fix backtrace in blk_cleanup_queue
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
  2010-02-26 16:04 ` [PATCH 1 of 8] blktap: Fix tap kobj double unref in sysfs_destroy Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 3 of 8] blkfront: Stop leaking the gendisk struct Daniel Stodden
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 82 bytes --]

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org



[-- Attachment #2: fix-blk-cleanup.diff --]
[-- Type: text/x-patch, Size: 789 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267197952 28800
# Node ID 93086816e211834e57d2d0d425b9eb00d3140a8c
# Parent  3ea95de27eaed11b87a5c608629f5089b0b33a73
blkfront: Fix backtrace in blk_cleanup_queue.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org

diff -r 3ea95de27eae -r 93086816e211 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:51 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:52 2010 -0800
@@ -942,11 +942,11 @@
 	/* Flush gnttab callback work. Must be done with no locks held. */
 	flush_scheduled_work();
 
+	del_gendisk(info->gd);
+
 	blk_cleanup_queue(info->rq);
 	info->rq = NULL;
 
-	del_gendisk(info->gd);
-
  out:
 	xenbus_frontend_closed(dev);
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3 of 8] blkfront: Stop leaking the gendisk struct
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
  2010-02-26 16:04 ` [PATCH 1 of 8] blktap: Fix tap kobj double unref in sysfs_destroy Daniel Stodden
  2010-02-26 16:04 ` [PATCH 2 of 8] blkfront: Fix backtrace in blk_cleanup_queue Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 4 of 8] blkfront: Fix an unlikely xenbus crasher Daniel Stodden
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 82 bytes --]

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org



[-- Attachment #2: fix-gendisk-leak.diff --]
[-- Type: text/x-patch, Size: 676 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267197953 28800
# Node ID 3b7ebe4ce587d81361bf9afb5825420fa3d4c9d7
# Parent  93086816e211834e57d2d0d425b9eb00d3140a8c
blkfront: Stop leaking the gendisk struct.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org

diff -r 93086816e211 -r 3b7ebe4ce587 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:52 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:53 2010 -0800
@@ -947,6 +947,9 @@
 	blk_cleanup_queue(info->rq);
 	info->rq = NULL;
 
+	put_disk(info->gd);
+	info->gd = NULL;
+
  out:
 	xenbus_frontend_closed(dev);
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4 of 8] blkfront: Fix an unlikely xenbus crasher
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
                   ` (2 preceding siblings ...)
  2010-02-26 16:04 ` [PATCH 3 of 8] blkfront: Stop leaking the gendisk struct Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 5 of 8] blkfront: Clean up closing transition Daniel Stodden
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 154 bytes --]

Getting partition 0 should not fail, but if it does we won't continue.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org



[-- Attachment #2: fix-bd-oops.diff --]
[-- Type: text/x-patch, Size: 825 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267197954 28800
# Node ID fcefa69c0dbf52a86a9037ad14f81d3252f387ff
# Parent  3b7ebe4ce587d81361bf9afb5825420fa3d4c9d7
blkfront: Fix an unlikely xenbus crasher.

Getting partition 0 should not fail, but if it does we won't continue.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
CC: stable@kernel.org

diff -r 3b7ebe4ce587 -r fcefa69c0dbf drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:53 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:54 2010 -0800
@@ -985,8 +985,10 @@
 			break;
 		}
 		bd = bdget_disk(info->gd, 0);
-		if (bd == NULL)
+		if (bd == NULL) {
 			xenbus_dev_fatal(dev, -ENODEV, "bdget failed");
+			break;
+		}
 
 		mutex_lock(&bd->bd_mutex);
 		if (info->users > 0)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5 of 8] blkfront: Clean up closing transition
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
                   ` (3 preceding siblings ...)
  2010-02-26 16:04 ` [PATCH 4 of 8] blkfront: Fix an unlikely xenbus crasher Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 6 of 8] blkfront: Move xenstore accesses off the device release path Daniel Stodden
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]

Just gained enough locals to justify a proper subroutine.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Cc: stable@kernel.org



[-- Attachment #2: blkfront-close.diff --]
[-- Type: text/x-patch, Size: 1995 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267198064 28800
# Node ID 96525ab0393ab7cab181de757a495f57284ccd01
# Parent  fcefa69c0dbf52a86a9037ad14f81d3252f387ff
blkfront: Clean up closing transition.

Just gained enough locals to justify a proper subroutine.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Cc: stable@kernel.org

diff -r fcefa69c0dbf -r 96525ab0393a drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 07:25:54 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 07:27:44 2010 -0800
@@ -954,6 +954,35 @@
 	xenbus_frontend_closed(dev);
 }
 
+static void blkfront_close(struct blkfront_info *info)
+{
+	struct xenbus_device *dev = info->xbdev;
+	struct gendisk *gd = info->gd;
+	struct block_device *bd;
+
+	if (!gd) {
+		xenbus_frontend_closed(dev);
+		return;
+	}
+
+	bd = bdget_disk(gd, 0);
+	if (!bd) {
+		xenbus_dev_fatal(dev, -ENODEV, "bdget failed");
+		return;
+	}
+
+	mutex_lock(&bd->bd_mutex);
+
+	if (info->users > 0)
+		xenbus_dev_error(dev, -EBUSY,
+				 "Device in use; refusing to close");
+	else
+		blkfront_closing(dev);
+
+	mutex_unlock(&bd->bd_mutex);
+	bdput(bd);
+}
+
 /**
  * Callback received when the backend's state changes.
  */
@@ -961,7 +990,6 @@
 			    enum xenbus_state backend_state)
 {
 	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
-	struct block_device *bd;
 
 	dev_dbg(&dev->dev, "blkfront:blkback_changed to state %d.\n", backend_state);
 
@@ -980,24 +1008,7 @@
 		break;
 
 	case XenbusStateClosing:
-		if (info->gd == NULL) {
-			xenbus_frontend_closed(dev);
-			break;
-		}
-		bd = bdget_disk(info->gd, 0);
-		if (bd == NULL) {
-			xenbus_dev_fatal(dev, -ENODEV, "bdget failed");
-			break;
-		}
-
-		mutex_lock(&bd->bd_mutex);
-		if (info->users > 0)
-			xenbus_dev_error(dev, -EBUSY,
-					 "Device in use; refusing to close");
-		else
-			blkfront_closing(dev);
-		mutex_unlock(&bd->bd_mutex);
-		bdput(bd);
+		blkfront_close(info);
 		break;
 	}
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6 of 8] blkfront: Move xenstore accesses off the device release path
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
                   ` (4 preceding siblings ...)
  2010-02-26 16:04 ` [PATCH 5 of 8] blkfront: Clean up closing transition Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 7 of 8] blkfront: Clean up blkfront_closing Daniel Stodden
  2010-02-26 16:04 ` [PATCH 8 of 8] blkfront: Clean up debug statements Daniel Stodden
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 191 bytes --]

Not holding the bdev mutex should be slightly more robust. Also helps
getting blkfront_closing to complement xlvbd_alloc_gendisk.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>



[-- Attachment #2: xs-write-unlocked.diff --]
[-- Type: text/x-patch, Size: 1948 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267200019 28800
# Node ID 183cbdaef8d14d863465c18bc06cbc7849c30bbc
# Parent  96525ab0393ab7cab181de757a495f57284ccd01
blkfront: Move xenstore accesses off the device release path.

Not holding the bdev mutex should be slightly more robust. Also helps
getting blkfront_closing to complement xlvbd_alloc_gendisk.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>

diff -r 96525ab0393a -r 183cbdaef8d1 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 07:27:44 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 08:00:19 2010 -0800
@@ -927,8 +927,8 @@
 
 	dev_dbg(&dev->dev, "blkfront_closing: %s removed\n", dev->nodename);
 
-	if (info->rq == NULL)
-		goto out;
+	if (!info->gd)
+		return;
 
 	spin_lock_irqsave(&blkif_io_lock, flags);
 
@@ -949,9 +949,6 @@
 
 	put_disk(info->gd);
 	info->gd = NULL;
-
- out:
-	xenbus_frontend_closed(dev);
 }
 
 static void blkfront_close(struct blkfront_info *info)
@@ -959,6 +956,7 @@
 	struct xenbus_device *dev = info->xbdev;
 	struct gendisk *gd = info->gd;
 	struct block_device *bd;
+	int inuse;
 
 	if (!gd) {
 		xenbus_frontend_closed(dev);
@@ -973,14 +971,18 @@
 
 	mutex_lock(&bd->bd_mutex);
 
-	if (info->users > 0)
-		xenbus_dev_error(dev, -EBUSY,
-				 "Device in use; refusing to close");
-	else
+	inuse = !!info->users;
+	if (!inuse)
 		blkfront_closing(dev);
 
 	mutex_unlock(&bd->bd_mutex);
 	bdput(bd);
+
+	if (inuse)
+		xenbus_dev_error(dev, -EBUSY,
+				 "Device in use; refusing to close");
+	else
+		xenbus_frontend_closed(dev);
 }
 
 /**
@@ -1051,8 +1053,10 @@
 		struct xenbus_device *dev = info->xbdev;
 		enum xenbus_state state = xenbus_read_driver_state(dev->otherend);
 
-		if (state == XenbusStateClosing && info->is_ready)
+		if (state == XenbusStateClosing && info->is_ready) {
 			blkfront_closing(dev);
+			xenbus_frontend_closed(dev);
+		}
 	}
 	return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 7 of 8] blkfront: Clean up blkfront_closing
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
                   ` (5 preceding siblings ...)
  2010-02-26 16:04 ` [PATCH 6 of 8] blkfront: Move xenstore accesses off the device release path Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  2010-02-26 16:04 ` [PATCH 8 of 8] blkfront: Clean up debug statements Daniel Stodden
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

* Call blkfront_closing the xlvbd_release_gendisk it is.
 * Fix comments regarding device disconnects.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Cc: stable@kernel.org



[-- Attachment #2: xlvbd-release-gendisk.diff --]
[-- Type: text/x-patch, Size: 3164 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267200152 28800
# Node ID 03ff587d68cd4146470e874c587250d9bc7a2492
# Parent  183cbdaef8d14d863465c18bc06cbc7849c30bbc
blkfront: Clean up blkfront_closing.

 * Call blkfront_closing the xlvbd_release_gendisk it is.
 * Fix comments regarding device disconnects.

Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Cc: stable@kernel.org

diff -r 183cbdaef8d1 -r 03ff587d68cd drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 08:00:19 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 08:02:32 2010 -0800
@@ -473,6 +473,37 @@
 	return err;
 }
 
+static void xlvbd_release_gendisk(struct blkfront_info *info)
+{
+	struct xenbus_device *dev = info->xbdev;
+	unsigned long flags;
+
+	dev_dbg(&dev->dev, "%s: %s removed\n", __func__, dev->nodename);
+
+	if (!info->gd)
+		return;
+
+	spin_lock_irqsave(&blkif_io_lock, flags);
+
+	/* No more blkif_request(). */
+	blk_stop_queue(info->rq);
+
+	/* No more gnttab callback work. */
+	gnttab_cancel_free_callback(&info->callback);
+	spin_unlock_irqrestore(&blkif_io_lock, flags);
+
+	/* Flush gnttab callback work. Must be done with no locks held. */
+	flush_scheduled_work();
+
+	del_gendisk(info->gd);
+
+	blk_cleanup_queue(info->rq);
+	info->rq = NULL;
+
+	put_disk(info->gd);
+	info->gd = NULL;
+}
+
 static void kick_pending_request_queues(struct blkfront_info *info)
 {
 	if (!RING_FULL(&info->ring)) {
@@ -915,42 +946,10 @@
 }
 
 /**
- * Handle the change of state of the backend to Closing.  We must delete our
- * device-layer structures now, to ensure that writes are flushed through to
- * the backend.  Once is this done, we can switch to Closed in
- * acknowledgement.
+ * Handle the change of state of the backend to Closing. Refuse to
+ * disconnect while the device is still in use. This also ensures that
+ * writes are flushed through to the backend.
  */
-static void blkfront_closing(struct xenbus_device *dev)
-{
-	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
-	unsigned long flags;
-
-	dev_dbg(&dev->dev, "blkfront_closing: %s removed\n", dev->nodename);
-
-	if (!info->gd)
-		return;
-
-	spin_lock_irqsave(&blkif_io_lock, flags);
-
-	/* No more blkif_request(). */
-	blk_stop_queue(info->rq);
-
-	/* No more gnttab callback work. */
-	gnttab_cancel_free_callback(&info->callback);
-	spin_unlock_irqrestore(&blkif_io_lock, flags);
-
-	/* Flush gnttab callback work. Must be done with no locks held. */
-	flush_scheduled_work();
-
-	del_gendisk(info->gd);
-
-	blk_cleanup_queue(info->rq);
-	info->rq = NULL;
-
-	put_disk(info->gd);
-	info->gd = NULL;
-}
-
 static void blkfront_close(struct blkfront_info *info)
 {
 	struct xenbus_device *dev = info->xbdev;
@@ -973,7 +972,7 @@
 
 	inuse = !!info->users;
 	if (!inuse)
-		blkfront_closing(dev);
+		xlvbd_release_gendisk(info);
 
 	mutex_unlock(&bd->bd_mutex);
 	bdput(bd);
@@ -1054,7 +1053,7 @@
 		enum xenbus_state state = xenbus_read_driver_state(dev->otherend);
 
 		if (state == XenbusStateClosing && info->is_ready) {
-			blkfront_closing(dev);
+			xlvbd_release_gendisk(info);
 			xenbus_frontend_closed(dev);
 		}
 	}

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 8 of 8] blkfront: Clean up debug statements
  2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
                   ` (6 preceding siblings ...)
  2010-02-26 16:04 ` [PATCH 7 of 8] blkfront: Clean up blkfront_closing Daniel Stodden
@ 2010-02-26 16:04 ` Daniel Stodden
  7 siblings, 0 replies; 9+ messages in thread
From: Daniel Stodden @ 2010-02-26 16:04 UTC (permalink / raw)
  To: Xen; +Cc: Jeremy Fitzhardinge

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

* Use dev_dbg everywhere.
 * Fix excess line withs.
 * Wrap dev_dbg to take a blkfront_info handle.
 * Separate datapath debug statements (DEV_QDBG), conditionally.
 * Verbose disk remove/release dbg_info statements.



[-- Attachment #2: debug-common.diff --]
[-- Type: text/x-patch, Size: 7120 bytes --]

# HG changeset patch
# User Daniel Stodden <daniel.stodden@citrix.com>
# Date 1267200155 28800
# Node ID 03b5c624c6186447675e996b62747e6aa87671a4
# Parent  03ff587d68cd4146470e874c587250d9bc7a2492
blkfront: Clean up debug statements.

 * Use dev_dbg everywhere.
 * Fix excess line withs.
 * Wrap dev_dbg to take a blkfront_info handle.
 * Separate datapath debug statements (DEV_QDBG), conditionally.
 * Verbose disk remove/release dbg_info statements.

diff -r 03ff587d68cd -r 03b5c624c618 drivers/block/xen-blkfront.c
--- a/drivers/block/xen-blkfront.c	Fri Feb 26 08:02:32 2010 -0800
+++ b/drivers/block/xen-blkfront.c	Fri Feb 26 08:02:35 2010 -0800
@@ -35,6 +35,8 @@
  * IN THE SOFTWARE.
  */
 
+/* #define DEBUG 1 */
+
 #include <linux/interrupt.h>
 #include <linux/blkdev.h>
 #include <linux/hdreg.h>
@@ -100,6 +102,19 @@
 	int users;
 };
 
+#define DEV_DBG(_info, _fmt, _args ...) \
+	dev_dbg(&(_info)->xbdev->dev, "%s: "_fmt, __func__, ##_args)
+#define DEV_WARN(_info, _fmt, _args ...) \
+	dev_warn(&(_info)->xbdev->dev, _fmt, ##_args)
+#define DEV_INFO(_info, _fmt, _args ...) \
+	dev_warn(&(_info)->xbdev->dev, _fmt, ##_args)
+
+#if defined(DEBUG) && DEBUG >= 2
+#define DEV_QDBG(_info, _fmt, _args ...) DEV_DBG(_info, _fmt, ##_args)
+#else
+#define DEV_QDBG(_info, _fmt, _args ...) do {} while (0)
+#endif
+
 static DEFINE_SPINLOCK(blkif_io_lock);
 
 #define MAXIMUM_OUTSTANDING_BLOCK_REQS \
@@ -164,12 +179,11 @@
 	struct blkfront_info *info = bdev->bd_disk->private_data;
 	int i;
 
-	dev_dbg(&info->xbdev->dev, "command: 0x%x, argument: 0x%lx\n",
-		command, (long)argument);
+	DEV_DBG(info, "command: 0x%x, argument: 0x%lx\n", command, (long)argument);
 
 	switch (command) {
 	case CDROMMULTISESSION:
-		dev_dbg(&info->xbdev->dev, "FIXME: support multisession CDs later\n");
+		DEV_DBG(info, "FIXME: support multisession CDs later\n");
 		for (i = 0; i < sizeof(struct cdrom_multisession); i++)
 			if (put_user(0, (char __user *)(argument + i)))
 				return -EFAULT;
@@ -183,8 +197,7 @@
 	}
 
 	default:
-		/*printk(KERN_ALERT "ioctl %08x not supported by Xen blkdev\n",
-		  command);*/
+		/* DEV_WARN(info, "ioctl %08x not supported\n", command); */
 		return -EINVAL; /* same return as native Linux */
 	}
 
@@ -291,16 +304,15 @@
  */
 static void do_blkif_request(struct request_queue *rq)
 {
-	struct blkfront_info *info = NULL;
+	struct blkfront_info *info = rq->queuedata;
 	struct request *req;
 	int queued;
 
-	pr_debug("Entered do_blkif_request\n");
+	DEV_QDBG(info, "\n");
 
 	queued = 0;
 
 	while ((req = blk_peek_request(rq)) != NULL) {
-		info = req->rq_disk->private_data;
 
 		if (RING_FULL(&info->ring))
 			goto wait;
@@ -312,8 +324,8 @@
 			continue;
 		}
 
-		pr_debug("do_blk_req %p: cmd %p, sec %lx, "
-			 "(%u/%u) buffer:%p [%s]\n",
+		DEV_QDBG(info,
+			 "%p: cmd %p, sec %lx, (%u/%u) buffer:%p [%s]\n",
 			 req, req->cmd, (unsigned long)blk_rq_pos(req),
 			 blk_rq_cur_sectors(req), blk_rq_sectors(req),
 			 req->buffer, rq_data_dir(req) ? "write" : "read");
@@ -378,9 +390,6 @@
 	if (err)
 		return err;
 
-	printk(KERN_INFO "blkfront: %s: barriers %s\n",
-	       info->gd->disk_name,
-	       info->feature_barrier ? "enabled" : "disabled");
 	return 0;
 }
 
@@ -401,7 +410,8 @@
 
 	if ((info->vdevice>>EXT_SHIFT) > 1) {
 		/* this is above the extended range; something is wrong */
-		printk(KERN_WARNING "blkfront: vdevice 0x%x is above the extended range; ignoring\n", info->vdevice);
+		DEV_WARN(info, "vdevice %#x is above %#x; ignoring\n",
+			 info->vdevice, 1U<<EXT_SHIFT);
 		return -ENODEV;
 	}
 
@@ -452,8 +462,9 @@
 		goto out;
 	}
 
+	info->gd = gd;
 	info->rq = gd->queue;
-	info->gd = gd;
+	info->rq->queuedata = info;
 
 	if (info->feature_barrier)
 		xlvbd_barrier(info);
@@ -475,11 +486,8 @@
 
 static void xlvbd_release_gendisk(struct blkfront_info *info)
 {
-	struct xenbus_device *dev = info->xbdev;
 	unsigned long flags;
 
-	dev_dbg(&dev->dev, "%s: %s removed\n", __func__, dev->nodename);
-
 	if (!info->gd)
 		return;
 
@@ -595,8 +603,7 @@
 		switch (bret->operation) {
 		case BLKIF_OP_WRITE_BARRIER:
 			if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
-				printk(KERN_WARNING "blkfront: %s: write barrier op failed\n",
-				       info->gd->disk_name);
+				DEV_WARN(info, "write barrier op failed\n");
 				error = -EOPNOTSUPP;
 				info->feature_barrier = 0;
 				xlvbd_barrier(info);
@@ -605,8 +612,9 @@
 		case BLKIF_OP_READ:
 		case BLKIF_OP_WRITE:
 			if (unlikely(bret->status != BLKIF_RSP_OKAY))
-				dev_dbg(&info->xbdev->dev, "Bad return from blkdev data "
-					"request: %x\n", bret->status);
+				DEV_QDBG(info,
+					 "Bad return from blkdev data "
+					 "request: %x\n", bret->status);
 
 			__blk_end_request_all(req, error);
 			break;
@@ -877,7 +885,7 @@
 	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
 	int err;
 
-	dev_dbg(&dev->dev, "blkfront_resume: %s\n", dev->nodename);
+	DEV_DBG(info, "connected=%d\n", info->connected);
 
 	blkif_free(info, info->connected == BLKIF_STATE_CONNECTED);
 
@@ -904,8 +912,7 @@
 	    (info->connected == BLKIF_STATE_SUSPENDED) )
 		return;
 
-	dev_dbg(&info->xbdev->dev, "%s:%s.\n",
-		__func__, info->xbdev->otherend);
+	DEV_DBG(info, "connecting to %s\n", info->xbdev->otherend);
 
 	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
 			    "sectors", "%llu", &sectors,
@@ -940,6 +947,9 @@
 	kick_pending_request_queues(info);
 	spin_unlock_irq(&blkif_io_lock);
 
+	DEV_INFO(info, "%s: %llu MiB, %luB sectors, barriers %s\n",
+		 info->gd->disk_name, (sectors * sector_size) >> 20,
+		 sector_size, info->feature_barrier ? "enabled" : "disabled");
 	add_disk(info->gd);
 
 	info->is_ready = 1;
@@ -971,8 +981,10 @@
 	mutex_lock(&bd->bd_mutex);
 
 	inuse = !!info->users;
-	if (!inuse)
+	if (!inuse) {
+		get_disk(gd);
 		xlvbd_release_gendisk(info);
+	}
 
 	mutex_unlock(&bd->bd_mutex);
 	bdput(bd);
@@ -980,8 +992,11 @@
 	if (inuse)
 		xenbus_dev_error(dev, -EBUSY,
 				 "Device in use; refusing to close");
-	else
+	else {
+		DEV_INFO(info, "%s removed\n", gd->disk_name);
+		put_disk(gd);
 		xenbus_frontend_closed(dev);
+	}
 }
 
 /**
@@ -992,7 +1007,7 @@
 {
 	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
 
-	dev_dbg(&dev->dev, "blkfront:blkback_changed to state %d.\n", backend_state);
+	DEV_DBG(info, "state %d\n", backend_state);
 
 	switch (backend_state) {
 	case XenbusStateInitialising:
@@ -1018,7 +1033,7 @@
 {
 	struct blkfront_info *info = dev_get_drvdata(&dev->dev);
 
-	dev_dbg(&dev->dev, "blkfront_remove: %s removed\n", dev->nodename);
+	DEV_DBG(info, "\n");
 
 	blkif_free(info, 0);
 
@@ -1088,13 +1103,16 @@
 
 static int __init xlblk_init(void)
 {
+	int err;
+
 	if (!xen_domain())
 		return -ENODEV;
 
-	if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) {
-		printk(KERN_WARNING "xen_blk: can't get major %d with name %s\n",
-		       XENVBD_MAJOR, DEV_NAME);
-		return -ENODEV;
+	err = register_blkdev(XENVBD_MAJOR, DEV_NAME);
+	if (err) {
+		printk(KERN_WARNING "%s: can't get major %d with name %s: %d\n",
+		       KBUILD_MODNAME, XENVBD_MAJOR, DEV_NAME, err);
+		return err;
 	}
 
 	return xenbus_register_frontend(&blkfront);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-02-26 16:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-26 16:04 [PATCH 0 of 8] Linux/pvops blktap + blkfront updates Daniel Stodden
2010-02-26 16:04 ` [PATCH 1 of 8] blktap: Fix tap kobj double unref in sysfs_destroy Daniel Stodden
2010-02-26 16:04 ` [PATCH 2 of 8] blkfront: Fix backtrace in blk_cleanup_queue Daniel Stodden
2010-02-26 16:04 ` [PATCH 3 of 8] blkfront: Stop leaking the gendisk struct Daniel Stodden
2010-02-26 16:04 ` [PATCH 4 of 8] blkfront: Fix an unlikely xenbus crasher Daniel Stodden
2010-02-26 16:04 ` [PATCH 5 of 8] blkfront: Clean up closing transition Daniel Stodden
2010-02-26 16:04 ` [PATCH 6 of 8] blkfront: Move xenstore accesses off the device release path Daniel Stodden
2010-02-26 16:04 ` [PATCH 7 of 8] blkfront: Clean up blkfront_closing Daniel Stodden
2010-02-26 16:04 ` [PATCH 8 of 8] blkfront: Clean up debug statements Daniel Stodden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).