* [PATCH 02/16] blkfront: fixes for 'xm block-detach ... --force'
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 03/16] blkfront: don't access freed struct xenbus_device Jeremy Fitzhardinge
` (13 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe; +Cc: Xen-devel, Linux Kernel Mailing List, Jeremy Fitzhardinge
From: Jan Beulich <jbeulich@novell.com>
Prevent prematurely freeing 'struct blkfront_info' instances (when the
xenbus data structures are gone, but the Linux ones are still needed).
Prevent adding a disk with the same (major, minor) [and hence the same
name and sysfs entries, which leads to oopses] when the previous
instance wasn't fully de-allocated yet.
This still doesn't address all issues resulting from forced detach:
I/O submitted after the detach still blocks forever, likely preventing
subsequent un-mounting from completing. It's not clear to me (not
knowing much about the block layer) how this can be avoided.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 83 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 78 insertions(+), 5 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 6839947..b1b16e5 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -104,6 +104,10 @@ struct blkfront_info
static DEFINE_SPINLOCK(blkif_io_lock);
+static unsigned int nr_minors;
+static unsigned long *minors;
+static DEFINE_SPINLOCK(minor_lock);
+
#define MAXIMUM_OUTSTANDING_BLOCK_REQS \
(BLKIF_MAX_SEGMENTS_PER_REQUEST * BLK_RING_SIZE)
#define GRANT_INVALID_REF 0
@@ -138,6 +142,55 @@ static void add_id_to_freelist(struct blkfront_info *info,
info->shadow_free = id;
}
+static int xlbd_reserve_minors(unsigned int minor, unsigned int nr)
+{
+ unsigned int end = minor + nr;
+ int rc;
+
+ if (end > nr_minors) {
+ unsigned long *bitmap, *old;
+
+ bitmap = kzalloc(BITS_TO_LONGS(end) * sizeof(*bitmap),
+ GFP_KERNEL);
+ if (bitmap == NULL)
+ return -ENOMEM;
+
+ spin_lock(&minor_lock);
+ if (end > nr_minors) {
+ old = minors;
+ memcpy(bitmap, minors,
+ BITS_TO_LONGS(nr_minors) * sizeof(*bitmap));
+ minors = bitmap;
+ nr_minors = BITS_TO_LONGS(end) * BITS_PER_LONG;
+ } else
+ old = bitmap;
+ spin_unlock(&minor_lock);
+ kfree(old);
+ }
+
+ spin_lock(&minor_lock);
+ if (find_next_bit(minors, end, minor) >= end) {
+ for (; minor < end; ++minor)
+ __set_bit(minor, minors);
+ rc = 0;
+ } else
+ rc = -EBUSY;
+ spin_unlock(&minor_lock);
+
+ return rc;
+}
+
+static void xlbd_release_minors(unsigned int minor, unsigned int nr)
+{
+ unsigned int end = minor + nr;
+
+ BUG_ON(end > nr_minors);
+ spin_lock(&minor_lock);
+ for (; minor < end; ++minor)
+ __clear_bit(minor, minors);
+ spin_unlock(&minor_lock);
+}
+
static void blkif_restart_queue_callback(void *arg)
{
struct blkfront_info *info = (struct blkfront_info *)arg;
@@ -417,9 +470,14 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
if ((minor % nr_parts) == 0)
nr_minors = nr_parts;
+ err = xlbd_reserve_minors(minor, nr_minors);
+ if (err)
+ goto out;
+ err = -ENODEV;
+
gd = alloc_disk(nr_minors);
if (gd == NULL)
- goto out;
+ goto release;
offset = minor / nr_parts;
@@ -450,7 +508,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
if (xlvbd_init_blk_queue(gd, sector_size)) {
del_gendisk(gd);
- goto out;
+ goto release;
}
info->rq = gd->queue;
@@ -470,6 +528,8 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
return 0;
+ release:
+ xlbd_release_minors(minor, nr_minors);
out:
return err;
}
@@ -924,6 +984,7 @@ static void blkfront_connect(struct blkfront_info *info)
static void blkfront_closing(struct xenbus_device *dev)
{
struct blkfront_info *info = dev_get_drvdata(&dev->dev);
+ unsigned int minor, nr_minors;
unsigned long flags;
dev_dbg(&dev->dev, "blkfront_closing: %s removed\n", dev->nodename);
@@ -946,7 +1007,10 @@ static void blkfront_closing(struct xenbus_device *dev)
blk_cleanup_queue(info->rq);
info->rq = NULL;
+ minor = info->gd->first_minor;
+ nr_minors = info->gd->minors;
del_gendisk(info->gd);
+ xlbd_release_minors(minor, nr_minors);
out:
xenbus_frontend_closed(dev);
@@ -1004,7 +1068,10 @@ static int blkfront_remove(struct xenbus_device *dev)
blkif_free(info, 0);
- kfree(info);
+ if(info->users == 0)
+ kfree(info);
+ else
+ info->is_ready = -1;
return 0;
}
@@ -1013,12 +1080,15 @@ static int blkfront_is_ready(struct xenbus_device *dev)
{
struct blkfront_info *info = dev_get_drvdata(&dev->dev);
- return info->is_ready;
+ return info->is_ready > 0;
}
static int blkif_open(struct block_device *bdev, fmode_t mode)
{
struct blkfront_info *info = bdev->bd_disk->private_data;
+
+ if(info->is_ready < 0)
+ return -ENODEV;
info->users++;
return 0;
}
@@ -1034,7 +1104,10 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
struct xenbus_device *dev = info->xbdev;
enum xenbus_state state = xenbus_read_driver_state(dev->otherend);
- if (state == XenbusStateClosing && info->is_ready)
+ if(info->is_ready < 0) {
+ blkfront_closing(dev);
+ kfree(info);
+ } else if (state == XenbusStateClosing && info->is_ready)
blkfront_closing(dev);
}
return 0;
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/16] blkfront: don't access freed struct xenbus_device
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 02/16] blkfront: fixes for 'xm block-detach ... --force' Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 04/16] xen/front: Propagate changed size of VBDs Jeremy Fitzhardinge
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Jan Beulich, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Jan Beulich <jbeulich@novell.com>
Unfortunately commit "blkfront: fixes for 'xm block-detach ... --force'"
still wasn't quite right - there was a reference to freed memory left
from blkfront_closing().
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index b1b16e5..df2bae5 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -981,13 +981,11 @@ static void blkfront_connect(struct blkfront_info *info)
* the backend. Once is this done, we can switch to Closed in
* acknowledgement.
*/
-static void blkfront_closing(struct xenbus_device *dev)
+static void blkfront_closing(struct blkfront_info *info)
{
- struct blkfront_info *info = dev_get_drvdata(&dev->dev);
unsigned int minor, nr_minors;
unsigned long flags;
- dev_dbg(&dev->dev, "blkfront_closing: %s removed\n", dev->nodename);
if (info->rq == NULL)
goto out;
@@ -1013,7 +1011,8 @@ static void blkfront_closing(struct xenbus_device *dev)
xlbd_release_minors(minor, nr_minors);
out:
- xenbus_frontend_closed(dev);
+ if (info->xbdev)
+ xenbus_frontend_closed(info->xbdev);
}
/**
@@ -1053,7 +1052,7 @@ static void blkback_changed(struct xenbus_device *dev,
xenbus_dev_error(dev, -EBUSY,
"Device in use; refusing to close");
else
- blkfront_closing(dev);
+ blkfront_closing(info);
mutex_unlock(&bd->bd_mutex);
bdput(bd);
break;
@@ -1071,7 +1070,7 @@ static int blkfront_remove(struct xenbus_device *dev)
if(info->users == 0)
kfree(info);
else
- info->is_ready = -1;
+ info->xbdev = NULL;
return 0;
}
@@ -1080,14 +1079,14 @@ static int blkfront_is_ready(struct xenbus_device *dev)
{
struct blkfront_info *info = dev_get_drvdata(&dev->dev);
- return info->is_ready > 0;
+ return info->is_ready && info->xbdev;
}
static int blkif_open(struct block_device *bdev, fmode_t mode)
{
struct blkfront_info *info = bdev->bd_disk->private_data;
- if(info->is_ready < 0)
+ if (!info->xbdev)
return -ENODEV;
info->users++;
return 0;
@@ -1102,13 +1101,13 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
have ignored this request initially, as the device was
still mounted. */
struct xenbus_device *dev = info->xbdev;
- enum xenbus_state state = xenbus_read_driver_state(dev->otherend);
- if(info->is_ready < 0) {
- blkfront_closing(dev);
+ if (!dev) {
+ blkfront_closing(info);
kfree(info);
- } else if (state == XenbusStateClosing && info->is_ready)
- blkfront_closing(dev);
+ } else if (xenbus_read_driver_state(dev->otherend)
+ == XenbusStateClosing && info->is_ready)
+ blkfront_closing(info);
}
return 0;
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/16] xen/front: Propagate changed size of VBDs
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 02/16] blkfront: fixes for 'xm block-detach ... --force' Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 03/16] blkfront: don't access freed struct xenbus_device Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 05/16] xen/blkfront: avoid compiler warning from missing cases Jeremy Fitzhardinge
` (11 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, K. Y. Srinivasan, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: K. Y. Srinivasan <ksrinivasan@novell.com>
Support dynamic resizing of virtual block devices. This patch supports
both file backed block devices as well as physical devices that can be
dynamically resized on the host side.
Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index df2bae5..0b4091b 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -930,9 +930,24 @@ static void blkfront_connect(struct blkfront_info *info)
unsigned int binfo;
int err;
- if ((info->connected == BLKIF_STATE_CONNECTED) ||
- (info->connected == BLKIF_STATE_SUSPENDED) )
+ switch (info->connected) {
+ case BLKIF_STATE_CONNECTED:
+ /*
+ * Potentially, the back-end may be signalling
+ * a capacity change; update the capacity.
+ */
+ err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+ "sectors", "%Lu", §ors);
+ if (XENBUS_EXIST_ERR(err))
+ return;
+ printk(KERN_INFO "Setting capacity to %Lu\n",
+ sectors);
+ set_capacity(info->gd, sectors);
+
+ /* fall through */
+ case BLKIF_STATE_SUSPENDED:
return;
+ }
dev_dbg(&info->xbdev->dev, "%s:%s.\n",
__func__, info->xbdev->otherend);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/16] xen/blkfront: avoid compiler warning from missing cases
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (2 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 04/16] xen/front: Propagate changed size of VBDs Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 06/16] xen/blkfront: revalidate after setting capacity Jeremy Fitzhardinge
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe; +Cc: Xen-devel, Jeremy Fitzhardinge, Linux Kernel Mailing List
From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Fix:
drivers/block/xen-blkfront.c: In function ‘blkfront_connect’:
drivers/block/xen-blkfront.c:933: warning: enumeration value ‘BLKIF_STATE_DISCONNECTED’ not handled in switch
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 0b4091b..3426704 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -947,6 +947,9 @@ static void blkfront_connect(struct blkfront_info *info)
/* fall through */
case BLKIF_STATE_SUSPENDED:
return;
+
+ default:
+ break;
}
dev_dbg(&info->xbdev->dev, "%s:%s.\n",
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/16] xen/blkfront: revalidate after setting capacity
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (3 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 05/16] xen/blkfront: avoid compiler warning from missing cases Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 07/16] xenbus: Make xenbus_switch_state transactional Jeremy Fitzhardinge
` (9 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, K. Y. Srinivasan, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: K. Y. Srinivasan <ksrinivasan@novell.com>
Signed-off-by: K. Y. Srinivasan <ksrinivasan@novell.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 3426704..50b2952 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -943,6 +943,7 @@ static void blkfront_connect(struct blkfront_info *info)
printk(KERN_INFO "Setting capacity to %Lu\n",
sectors);
set_capacity(info->gd, sectors);
+ revalidate_disk(info->gd);
/* fall through */
case BLKIF_STATE_SUSPENDED:
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/16] xenbus: Make xenbus_switch_state transactional
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (4 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 06/16] xen/blkfront: revalidate after setting capacity Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 08/16] blkfront: Fix backtrace in del_gendisk Jeremy Fitzhardinge
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
According to the comments, this was how it's been done years ago, but
apparently took an xbt pointer from elsewhere back then. The code was
removed because of consistency issues: cancellation wont't roll back
the saved xbdev->state.
Still, unsolicited writes to the state field remain an issue,
especially if device shutdown takes thread synchronization, and subtle
races cause accidental recreation of the device node.
Fixed by reintroducing the transaction. An internal one is sufficient,
so the xbdev->state value remains consistent.
Also fixes the original hack to prevent infinite recursion. Instead of
bailing out on the first attempt to switch to Closing, checks call
depth now.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/xen/xenbus/xenbus_client.c | 90 ++++++++++++++++++++++++++----------
1 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 7b3e973..7e49527 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -133,17 +133,12 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev,
}
EXPORT_SYMBOL_GPL(xenbus_watch_pathfmt);
+static void xenbus_switch_fatal(struct xenbus_device *, int, int,
+ const char *, ...);
-/**
- * xenbus_switch_state
- * @dev: xenbus device
- * @state: new state
- *
- * Advertise in the store a change of the given driver to the given new_state.
- * Return 0 on success, or -errno on error. On error, the device will switch
- * to XenbusStateClosing, and the error will be saved in the store.
- */
-int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
+static int
+__xenbus_switch_state(struct xenbus_device *dev,
+ enum xenbus_state state, int depth)
{
/* We check whether the state is currently set to the given value, and
if not, then the state is set. We don't want to unconditionally
@@ -152,35 +147,65 @@ int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
to it, as the device will be tearing down, and we don't want to
resurrect that directory.
- Note that, because of this cached value of our state, this function
- will not work inside a Xenstore transaction (something it was
- trying to in the past) because dev->state would not get reset if
- the transaction was aborted.
-
+ Note that, because of this cached value of our state, this
+ function will not take a caller's Xenstore transaction
+ (something it was trying to in the past) because dev->state
+ would not get reset if the transaction was aborted.
*/
+ struct xenbus_transaction xbt;
int current_state;
- int err;
+ int err, abort;
if (state == dev->state)
return 0;
- err = xenbus_scanf(XBT_NIL, dev->nodename, "state", "%d",
- ¤t_state);
- if (err != 1)
+again:
+ abort = 1;
+
+ err = xenbus_transaction_start(&xbt);
+ if (err) {
+ xenbus_switch_fatal(dev, depth, err, "starting transaction");
return 0;
+ }
+
+ err = xenbus_scanf(xbt, dev->nodename, "state", "%d", ¤t_state);
+ if (err != 1)
+ goto abort;
- err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", state);
+ err = xenbus_printf(xbt, dev->nodename, "state", "%d", state);
if (err) {
- if (state != XenbusStateClosing) /* Avoid looping */
- xenbus_dev_fatal(dev, err, "writing new state");
- return err;
+ xenbus_switch_fatal(dev, depth, err, "writing new state");
+ goto abort;
}
- dev->state = state;
+ abort = 0;
+abort:
+ err = xenbus_transaction_end(xbt, abort);
+ if (err) {
+ if (err == -EAGAIN && !abort)
+ goto again;
+ xenbus_switch_fatal(dev, depth, err, "ending transaction");
+ } else
+ dev->state = state;
return 0;
}
+
+/**
+ * xenbus_switch_state
+ * @dev: xenbus device
+ * @state: new state
+ *
+ * Advertise in the store a change of the given driver to the given new_state.
+ * Return 0 on success, or -errno on error. On error, the device will switch
+ * to XenbusStateClosing, and the error will be saved in the store.
+ */
+int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
+{
+ return __xenbus_switch_state(dev, state, 0);
+}
+
EXPORT_SYMBOL_GPL(xenbus_switch_state);
int xenbus_frontend_closed(struct xenbus_device *dev)
@@ -284,6 +309,23 @@ void xenbus_dev_fatal(struct xenbus_device *dev, int err, const char *fmt, ...)
EXPORT_SYMBOL_GPL(xenbus_dev_fatal);
/**
+ * Equivalent to xenbus_dev_fatal(dev, err, fmt, args), but helps
+ * avoiding recursion within xenbus_switch_state.
+ */
+static void xenbus_switch_fatal(struct xenbus_device *dev, int depth, int err,
+ const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ xenbus_va_dev_error(dev, err, fmt, ap);
+ va_end(ap);
+
+ if (!depth)
+ __xenbus_switch_state(dev, XenbusStateClosing, 1);
+}
+
+/**
* xenbus_grant_ring
* @dev: xenbus device
* @ring_mfn: mfn of ring to grant
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/16] blkfront: Fix backtrace in del_gendisk
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (5 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 07/16] xenbus: Make xenbus_switch_state transactional Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 09/16] blkfront: Fix gendisk leak Jeremy Fitzhardinge
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
The call to del_gendisk follows an non-refcounted gd->queue
pointer. We release the last ref in blk_cleanup_queue. Fixed by
reordering releases accordingly.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 50b2952..62eba76 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1021,14 +1021,14 @@ static void blkfront_closing(struct blkfront_info *info)
/* Flush gnttab callback work. Must be done with no locks held. */
flush_scheduled_work();
- blk_cleanup_queue(info->rq);
- info->rq = NULL;
-
minor = info->gd->first_minor;
nr_minors = info->gd->minors;
del_gendisk(info->gd);
xlbd_release_minors(minor, nr_minors);
+ blk_cleanup_queue(info->rq);
+ info->rq = NULL;
+
out:
if (info->xbdev)
xenbus_frontend_closed(info->xbdev);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/16] blkfront: Fix gendisk leak
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (6 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 08/16] blkfront: Fix backtrace in del_gendisk Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 10/16] blkfront: Clean up vbd release Jeremy Fitzhardinge
` (6 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 62eba76..c004ff0 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1029,6 +1029,9 @@ static void blkfront_closing(struct blkfront_info *info)
blk_cleanup_queue(info->rq);
info->rq = NULL;
+ put_disk(info->gd);
+ info->gd = NULL;
+
out:
if (info->xbdev)
xenbus_frontend_closed(info->xbdev);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/16] blkfront: Clean up vbd release
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (7 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 09/16] blkfront: Fix gendisk leak Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 11/16] blkfront: Lock blkfront_info when closing Jeremy Fitzhardinge
` (5 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
* Current blkfront_closing is rather a xlvbd_release_gendisk.
Renamed in preparation of later patches (need the name again).
* Removed the misleading comment -- this only applied to the backend
switch handler, and the queue is already flushed btw.
* Break out the xenbus call, callers know better when to switch
frontend state.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 91 ++++++++++++++++++++----------------------
1 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index c004ff0..7b03b48 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -534,6 +534,39 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
return err;
}
+static void xlvbd_release_gendisk(struct blkfront_info *info)
+{
+ unsigned int minor, nr_minors;
+ unsigned long flags;
+
+ if (info->rq == NULL)
+ 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);
+
+ minor = info->gd->first_minor;
+ nr_minors = info->gd->minors;
+ xlbd_release_minors(minor, nr_minors);
+
+ 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)) {
@@ -995,49 +1028,6 @@ static void blkfront_connect(struct blkfront_info *info)
}
/**
- * 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.
- */
-static void blkfront_closing(struct blkfront_info *info)
-{
- unsigned int minor, nr_minors;
- unsigned long flags;
-
-
- if (info->rq == NULL)
- goto out;
-
- 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();
-
- minor = info->gd->first_minor;
- nr_minors = info->gd->minors;
- del_gendisk(info->gd);
- xlbd_release_minors(minor, nr_minors);
-
- blk_cleanup_queue(info->rq);
- info->rq = NULL;
-
- put_disk(info->gd);
- info->gd = NULL;
-
- out:
- if (info->xbdev)
- xenbus_frontend_closed(info->xbdev);
-}
-
-/**
* Callback received when the backend's state changes.
*/
static void blkback_changed(struct xenbus_device *dev,
@@ -1073,8 +1063,11 @@ static void blkback_changed(struct xenbus_device *dev,
if (info->users > 0)
xenbus_dev_error(dev, -EBUSY,
"Device in use; refusing to close");
- else
- blkfront_closing(info);
+ else {
+ xlvbd_release_gendisk(info);
+ xenbus_frontend_closed(info->xbdev);
+ }
+
mutex_unlock(&bd->bd_mutex);
bdput(bd);
break;
@@ -1125,11 +1118,13 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
struct xenbus_device *dev = info->xbdev;
if (!dev) {
- blkfront_closing(info);
+ xlvbd_release_gendisk(info);
kfree(info);
} else if (xenbus_read_driver_state(dev->otherend)
- == XenbusStateClosing && info->is_ready)
- blkfront_closing(info);
+ == XenbusStateClosing && info->is_ready) {
+ xlvbd_release_gendisk(info);
+ xenbus_frontend_closed(dev);
+ }
}
return 0;
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/16] blkfront: Lock blkfront_info when closing
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (8 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 10/16] blkfront: Clean up vbd release Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 12/16] blkfront: Fix blkfront backend switch race (bdev open) Jeremy Fitzhardinge
` (4 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
The bdev .open/.release fops race against backend switches to Closing,
handled by the XenBus thread.
The original code attempted to serialize block device holders and
xenbus only via bd_mutex. This is insufficient, the info->bd pointer
may already be stale (or null) while xenbus tries to bump up the
refcount.
Protect blkfront_info with a dedicated mutex.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 61 +++++++++++++++++++++++++++--------------
1 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 7b03b48..1046a58 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -78,6 +78,7 @@ static const struct block_device_operations xlvbd_block_fops;
*/
struct blkfront_info
{
+ struct mutex mutex;
struct xenbus_device *xbdev;
struct gendisk *gd;
int vdevice;
@@ -804,7 +805,6 @@ again:
return err;
}
-
/**
* Entry point to this code when a new device is created. Allocate the basic
* structures and the ring buffer for communication with the backend, and
@@ -836,6 +836,7 @@ static int blkfront_probe(struct xenbus_device *dev,
return -ENOMEM;
}
+ mutex_init(&info->mutex);
info->xbdev = dev;
info->vdevice = vdevice;
info->connected = BLKIF_STATE_DISCONNECTED;
@@ -951,6 +952,43 @@ static int blkfront_resume(struct xenbus_device *dev)
return err;
}
+static void
+blkfront_closing(struct blkfront_info *info)
+{
+ struct xenbus_device *xbdev = info->xbdev;
+ struct block_device *bdev = NULL;
+
+ mutex_lock(&info->mutex);
+
+ if (xbdev->state == XenbusStateClosing) {
+ mutex_unlock(&info->mutex);
+ return;
+ }
+
+ if (info->gd)
+ bdev = bdget_disk(info->gd, 0);
+
+ mutex_unlock(&info->mutex);
+
+ if (!bdev) {
+ xenbus_frontend_closed(xbdev);
+ return;
+ }
+
+ mutex_lock(&bdev->bd_mutex);
+
+ if (info->users) {
+ xenbus_dev_error(xbdev, -EBUSY,
+ "Device in use; refusing to close");
+ xenbus_switch_state(xbdev, XenbusStateClosing);
+ } else {
+ xlvbd_release_gendisk(info);
+ xenbus_frontend_closed(xbdev);
+ }
+
+ mutex_unlock(&bdev->bd_mutex);
+ bdput(bdev);
+}
/*
* Invoked when the backend is finally 'ready' (and has told produced
@@ -1034,7 +1072,6 @@ static void blkback_changed(struct xenbus_device *dev,
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);
@@ -1051,25 +1088,7 @@ static void blkback_changed(struct xenbus_device *dev,
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");
-
- mutex_lock(&bd->bd_mutex);
- if (info->users > 0)
- xenbus_dev_error(dev, -EBUSY,
- "Device in use; refusing to close");
- else {
- xlvbd_release_gendisk(info);
- xenbus_frontend_closed(info->xbdev);
- }
-
- mutex_unlock(&bd->bd_mutex);
- bdput(bd);
+ blkfront_closing(info);
break;
}
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 12/16] blkfront: Fix blkfront backend switch race (bdev open)
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (9 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 11/16] blkfront: Lock blkfront_info when closing Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 13/16] blkfront: Fix blkfront backend switch race (bdev release) Jeremy Fitzhardinge
` (3 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
We need not mind if users grab a late handle on a closing disk. We
probably even should not. But we have to make sure it's not a dead
one already
Let the bdev deal with a gendisk deleted under its feet. Takes the
info mutex to decide a race against backend closing.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 25 ++++++++++++++++++++-----
1 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 1046a58..974d59a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1118,12 +1118,27 @@ static int blkfront_is_ready(struct xenbus_device *dev)
static int blkif_open(struct block_device *bdev, fmode_t mode)
{
- struct blkfront_info *info = bdev->bd_disk->private_data;
+ struct gendisk *disk = bdev->bd_disk;
+ struct blkfront_info *info;
+ int err = 0;
- if (!info->xbdev)
- return -ENODEV;
- info->users++;
- return 0;
+ info = disk->private_data;
+ if (!info)
+ /* xbdev gone */
+ return -ERESTARTSYS;
+
+ mutex_lock(&info->mutex);
+
+ if (!info->gd)
+ /* xbdev is closed */
+ err = -ERESTARTSYS;
+
+ mutex_unlock(&info->mutex);
+
+ if (!err)
+ ++info->users;
+
+ return err;
}
static int blkif_release(struct gendisk *disk, fmode_t mode)
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/16] blkfront: Fix blkfront backend switch race (bdev release)
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (10 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 12/16] blkfront: Fix blkfront backend switch race (bdev open) Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 14/16] blkfront: Lock blockfront_info during xbdev removal Jeremy Fitzhardinge
` (2 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
We cannot read backend state within bdev operations, because it risks
grabbing the state change before xenbus gets to do it.
Fixed by tracking deferral with a frontend switch to Closing. State
exposure isn't strictly necessary, but the backends won't mind.
For a 'clean' deferral this seems actually a more decent protocol than
raising errors.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 46 ++++++++++++++++++++++++++++-------------
1 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 974d59a..214c92e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1144,22 +1144,38 @@ static int blkif_open(struct block_device *bdev, fmode_t mode)
static int blkif_release(struct gendisk *disk, fmode_t mode)
{
struct blkfront_info *info = disk->private_data;
- info->users--;
- if (info->users == 0) {
- /* Check whether we have been instructed to close. We will
- have ignored this request initially, as the device was
- still mounted. */
- struct xenbus_device *dev = info->xbdev;
-
- if (!dev) {
- xlvbd_release_gendisk(info);
- kfree(info);
- } else if (xenbus_read_driver_state(dev->otherend)
- == XenbusStateClosing && info->is_ready) {
- xlvbd_release_gendisk(info);
- xenbus_frontend_closed(dev);
- }
+ struct block_device *bdev;
+ struct xenbus_device *xbdev;
+
+ if (--info->users)
+ return 0;
+
+ bdev = bdget_disk(disk, 0);
+ bdput(bdev);
+
+ /*
+ * Check if we have been instructed to close. We will have
+ * deferred this request, because the bdev was still open.
+ */
+
+ mutex_lock(&info->mutex);
+ xbdev = info->xbdev;
+
+ if (xbdev && xbdev->state == XenbusStateClosing) {
+ /* pending switch to state closed */
+ xlvbd_release_gendisk(info);
+ xenbus_frontend_closed(info->xbdev);
}
+
+ mutex_unlock(&info->mutex);
+
+ if (!xbdev) {
+ /* sudden device removal */
+ xlvbd_release_gendisk(info);
+ disk->private_data = NULL;
+ kfree(info);
+ }
+
return 0;
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 14/16] blkfront: Lock blockfront_info during xbdev removal
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (11 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 13/16] blkfront: Fix blkfront backend switch race (bdev release) Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 15/16] blkfront: Remove obsolete info->users Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 16/16] blkfront: Klog the unclean release path Jeremy Fitzhardinge
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
Same approach as blkfront_closing:
* Grab the bdev safely, holding the info mutex.
* Zap xbdev safely, holding the info mutex.
* Try bdev removal safely, holding bd_mutex.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 41 +++++++++++++++++++++++++++++++++++------
1 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 214c92e..96ff225 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1093,18 +1093,47 @@ static void blkback_changed(struct xenbus_device *dev,
}
}
-static int blkfront_remove(struct xenbus_device *dev)
+static int blkfront_remove(struct xenbus_device *xbdev)
{
- struct blkfront_info *info = dev_get_drvdata(&dev->dev);
+ struct blkfront_info *info = dev_get_drvdata(&xbdev->dev);
+ struct block_device *bdev = NULL;
+ struct gendisk *disk;
- dev_dbg(&dev->dev, "blkfront_remove: %s removed\n", dev->nodename);
+ dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
blkif_free(info, 0);
- if(info->users == 0)
+ mutex_lock(&info->mutex);
+
+ disk = info->gd;
+ if (disk)
+ bdev = bdget_disk(disk, 0);
+
+ info->xbdev = NULL;
+ mutex_unlock(&info->mutex);
+
+ if (!bdev) {
+ kfree(info);
+ return 0;
+ }
+
+ /*
+ * The xbdev was removed before we reached the Closed
+ * state. See if it's safe to remove the disk. If the bdev
+ * isn't closed yet, we let release take care of it.
+ */
+
+ mutex_lock(&bdev->bd_mutex);
+ info = disk->private_data;
+
+ if (info && !info->users) {
+ xlvbd_release_gendisk(info);
+ disk->private_data = NULL;
kfree(info);
- else
- info->xbdev = NULL;
+ }
+
+ mutex_unlock(&bdev->bd_mutex);
+ bdput(bdev);
return 0;
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 15/16] blkfront: Remove obsolete info->users
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (12 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 14/16] blkfront: Lock blockfront_info during xbdev removal Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
2010-07-20 20:42 ` [PATCH 16/16] blkfront: Klog the unclean release path Jeremy Fitzhardinge
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
This is just bd_openers, protected by the bd_mutex.
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 19 +++++--------------
1 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 96ff225..2c8de1b 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -95,12 +95,6 @@ struct blkfront_info
unsigned long shadow_free;
int feature_barrier;
int is_ready;
-
- /**
- * The number of people holding this device open. We won't allow a
- * hot-unplug unless this is 0.
- */
- int users;
};
static DEFINE_SPINLOCK(blkif_io_lock);
@@ -977,7 +971,7 @@ blkfront_closing(struct blkfront_info *info)
mutex_lock(&bdev->bd_mutex);
- if (info->users) {
+ if (bdev->bd_openers) {
xenbus_dev_error(xbdev, -EBUSY,
"Device in use; refusing to close");
xenbus_switch_state(xbdev, XenbusStateClosing);
@@ -1126,7 +1120,7 @@ static int blkfront_remove(struct xenbus_device *xbdev)
mutex_lock(&bdev->bd_mutex);
info = disk->private_data;
- if (info && !info->users) {
+ if (info && !bdev->bd_openers) {
xlvbd_release_gendisk(info);
disk->private_data = NULL;
kfree(info);
@@ -1164,9 +1158,6 @@ static int blkif_open(struct block_device *bdev, fmode_t mode)
mutex_unlock(&info->mutex);
- if (!err)
- ++info->users;
-
return err;
}
@@ -1176,12 +1167,12 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
struct block_device *bdev;
struct xenbus_device *xbdev;
- if (--info->users)
- return 0;
-
bdev = bdget_disk(disk, 0);
bdput(bdev);
+ if (bdev->bd_openers)
+ return 0;
+
/*
* Check if we have been instructed to close. We will have
* deferred this request, because the bdev was still open.
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 16/16] blkfront: Klog the unclean release path
2010-07-20 20:42 ` [PATCH 01/16] xen: use less generic names in blkfront driver Jeremy Fitzhardinge
` (13 preceding siblings ...)
2010-07-20 20:42 ` [PATCH 15/16] blkfront: Remove obsolete info->users Jeremy Fitzhardinge
@ 2010-07-20 20:42 ` Jeremy Fitzhardinge
14 siblings, 0 replies; 21+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-20 20:42 UTC (permalink / raw)
To: Jens Axboe
Cc: Xen-devel, Daniel Stodden, Linux Kernel Mailing List,
Jeremy Fitzhardinge
From: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Daniel Stodden <daniel.stodden@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
drivers/block/xen-blkfront.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2c8de1b..0b44dc1 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1120,6 +1120,10 @@ static int blkfront_remove(struct xenbus_device *xbdev)
mutex_lock(&bdev->bd_mutex);
info = disk->private_data;
+ dev_warn(disk_to_dev(disk),
+ "%s was hot-unplugged, %d stale handles\n",
+ xbdev->nodename, bdev->bd_openers);
+
if (info && !bdev->bd_openers) {
xlvbd_release_gendisk(info);
disk->private_data = NULL;
@@ -1183,6 +1187,7 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
if (xbdev && xbdev->state == XenbusStateClosing) {
/* pending switch to state closed */
+ dev_info(disk_to_dev(bdev->bd_disk), "releasing disk\n");
xlvbd_release_gendisk(info);
xenbus_frontend_closed(info->xbdev);
}
@@ -1191,6 +1196,7 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
if (!xbdev) {
/* sudden device removal */
+ dev_info(disk_to_dev(bdev->bd_disk), "releasing disk\n");
xlvbd_release_gendisk(info);
disk->private_data = NULL;
kfree(info);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 21+ messages in thread