* [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", §ors,
@@ -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).