* [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
@ 2014-05-09 23:34 Valentin Priescu
2014-05-09 23:56 ` Matt Wilson
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-09 23:34 UTC (permalink / raw)
To: xen-devel; +Cc: Valentin Priescu, Matt Wilson
From: Valentin Priescu <priescuv@amazon.com>
Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
requests to finish. If the VBD is attached to a hot-swappable disk, then
xenwatch can hang for a long period of time, stalling other watches.
INFO: task xenwatch:39 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
Call Trace:
[<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
[<ffffffff8128f761>] ? list_del+0x11/0x40
[<ffffffff8147a080>] ? wait_for_common+0x60/0x160
[<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
[<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
[<ffffffff8147a26a>] schedule+0x3a/0x60
[<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
[<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
[<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
[<ffffffff81346488>] device_release_driver+0x28/0x40
[<ffffffff813456e8>] bus_remove_device+0x78/0xe0
[<ffffffff81342c9f>] device_del+0x12f/0x1a0
[<ffffffff81342d2d>] device_unregister+0x1d/0x60
[<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
[<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
[<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
[<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
[<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
[<ffffffff8130b4cb>] frontend_changed+0xb/0x10
[<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
[<ffffffff810799d6>] kthread+0x96/0xa0
[<ffffffff81485934>] kernel_thread_helper+0x4/0x10
[<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
[<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
[<ffffffff81485930>] ? gs_change+0x13/0x13
With this patch, when there is still pending I/O, the actual disconnect
is done by the last reference holder (last pending I/O request). In this
case, xenwatch doesn't block indefinitely.
Signed-off-by: Valentin Priescu <priescuv@amazon.com>
Reviewed-by: Steven Kady <stevkady@amazon.com>
Reviewed-by: Steven Noonan <snoonan@amazon.com>
Cc: Matt Wilson <msw@amazon.com>
---
drivers/block/xen-blkback/common.h | 4 +--
drivers/block/xen-blkback/xenbus.c | 60 ++++++++++++++++++++++++++------------
2 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index be05277..f65b807 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -314,7 +314,7 @@ struct xen_blkif {
unsigned long long st_rd_sect;
unsigned long long st_wr_sect;
- wait_queue_head_t waiting_to_free;
+ struct work_struct free_work;
/* Thread shutdown wait queue. */
wait_queue_head_t shutdown_wq;
};
@@ -361,7 +361,7 @@ struct pending_req {
#define xen_blkif_put(_b) \
do { \
if (atomic_dec_and_test(&(_b)->refcnt)) \
- wake_up(&(_b)->waiting_to_free);\
+ schedule_work(&(_b)->free_work);\
} while (0)
struct phys_req {
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 9a547e6..7b6124e 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -35,12 +35,26 @@ static void connect(struct backend_info *);
static int connect_ring(struct backend_info *);
static void backend_changed(struct xenbus_watch *, const char **,
unsigned int);
+static void xen_blkif_free(struct xen_blkif *blkif);
+static void xen_vbd_free(struct xen_vbd *vbd);
struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
{
return be->dev;
}
+/*
+ * The last request could free the device from softirq context and
+ * xen_blkif_free() can sleep.
+ */
+static void xen_blkif_deferred_free(struct work_struct *work)
+{
+ struct xen_blkif *blkif;
+
+ blkif = container_of(work, struct xen_blkif, free_work);
+ xen_blkif_free(blkif);
+}
+
static int blkback_name(struct xen_blkif *blkif, char *buf)
{
char *devpath, *devname;
@@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
init_completion(&blkif->drain_complete);
atomic_set(&blkif->drain, 0);
blkif->st_print = jiffies;
- init_waitqueue_head(&blkif->waiting_to_free);
blkif->persistent_gnts.rb_node = NULL;
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
@@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
INIT_LIST_HEAD(&blkif->pending_free);
+ INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
for (i = 0; i < XEN_BLKIF_REQS; i++) {
req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
return 0;
}
-static void xen_blkif_disconnect(struct xen_blkif *blkif)
+static int xen_blkif_disconnect(struct xen_blkif *blkif)
{
if (blkif->xenblkd) {
kthread_stop(blkif->xenblkd);
@@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
blkif->xenblkd = NULL;
}
- atomic_dec(&blkif->refcnt);
- wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
- atomic_inc(&blkif->refcnt);
+ /* The above kthread_stop() guarantees that at this point we
+ * don't have any discard_io or other_io requests. So, checking
+ * for inflight IO is enough.
+ */
+ if (!atomic_read(&blkif->inflight)) {
+ if (blkif->irq) {
+ unbind_from_irqhandler(blkif->irq, blkif);
+ blkif->irq = 0;
+ }
- if (blkif->irq) {
- unbind_from_irqhandler(blkif->irq, blkif);
- blkif->irq = 0;
- }
+ if (blkif->blk_rings.common.sring) {
+ xenbus_unmap_ring_vfree(blkif->be->dev,
+ blkif->blk_ring);
+ blkif->blk_rings.common.sring = NULL;
+ }
- if (blkif->blk_rings.common.sring) {
- xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
- blkif->blk_rings.common.sring = NULL;
+ return 0;
}
+
+ return -EBUSY;
}
static void xen_blkif_free(struct xen_blkif *blkif)
@@ -259,8 +280,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
struct pending_req *req, *n;
int i = 0, j;
- if (!atomic_dec_and_test(&blkif->refcnt))
- BUG();
+ xen_blkif_disconnect(blkif);
+ xen_vbd_free(&blkif->vbd);
/* Remove all persistent grants and the cache of ballooned pages. */
xen_blkbk_free_caches(blkif);
@@ -449,16 +470,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
be->backend_watch.node = NULL;
}
+ dev_set_drvdata(&dev->dev, NULL);
+
if (be->blkif) {
xen_blkif_disconnect(be->blkif);
- xen_vbd_free(&be->blkif->vbd);
- xen_blkif_free(be->blkif);
- be->blkif = NULL;
+ xen_blkif_put(be->blkif);
}
kfree(be->mode);
kfree(be);
- dev_set_drvdata(&dev->dev, NULL);
return 0;
}
@@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
* Enforce precondition before potential leak point.
* xen_blkif_disconnect() is idempotent.
*/
- xen_blkif_disconnect(be->blkif);
+ err = xen_blkif_disconnect(be->blkif);
+ if (err)
+ break;
err = connect_ring(be);
if (err)
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-09 23:34 [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch Valentin Priescu
@ 2014-05-09 23:56 ` Matt Wilson
2014-05-12 6:42 ` Vasiliy Tolstov
2014-05-12 9:39 ` David Vrabel
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Matt Wilson @ 2014-05-09 23:56 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, Matt Wilson, Valentin Priescu
On Sat, May 10, 2014 at 01:34:39AM +0200, Valentin Priescu wrote:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
> INFO: task xenwatch:39 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
> Call Trace:
> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
> [<ffffffff8128f761>] ? list_del+0x11/0x40
> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
> [<ffffffff8147a26a>] schedule+0x3a/0x60
> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
> [<ffffffff81346488>] device_release_driver+0x28/0x40
> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
> [<ffffffff810799d6>] kthread+0x96/0xa0
> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
> [<ffffffff81485930>] ? gs_change+0x13/0x13
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
>
> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> Reviewed-by: Steven Kady <stevkady@amazon.com>
> Reviewed-by: Steven Noonan <snoonan@amazon.com>
> Cc: Matt Wilson <msw@amazon.com>
Acked-by: Matt Wilson <msw@amazon.com>
> ---
> drivers/block/xen-blkback/common.h | 4 +--
> drivers/block/xen-blkback/xenbus.c | 60 ++++++++++++++++++++++++++------------
> 2 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
> index be05277..f65b807 100644
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -314,7 +314,7 @@ struct xen_blkif {
> unsigned long long st_rd_sect;
> unsigned long long st_wr_sect;
>
> - wait_queue_head_t waiting_to_free;
> + struct work_struct free_work;
> /* Thread shutdown wait queue. */
> wait_queue_head_t shutdown_wq;
> };
> @@ -361,7 +361,7 @@ struct pending_req {
> #define xen_blkif_put(_b) \
> do { \
> if (atomic_dec_and_test(&(_b)->refcnt)) \
> - wake_up(&(_b)->waiting_to_free);\
> + schedule_work(&(_b)->free_work);\
> } while (0)
>
> struct phys_req {
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 9a547e6..7b6124e 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -35,12 +35,26 @@ static void connect(struct backend_info *);
> static int connect_ring(struct backend_info *);
> static void backend_changed(struct xenbus_watch *, const char **,
> unsigned int);
> +static void xen_blkif_free(struct xen_blkif *blkif);
> +static void xen_vbd_free(struct xen_vbd *vbd);
>
> struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
> {
> return be->dev;
> }
>
> +/*
> + * The last request could free the device from softirq context and
> + * xen_blkif_free() can sleep.
> + */
> +static void xen_blkif_deferred_free(struct work_struct *work)
> +{
> + struct xen_blkif *blkif;
> +
> + blkif = container_of(work, struct xen_blkif, free_work);
> + xen_blkif_free(blkif);
> +}
> +
> static int blkback_name(struct xen_blkif *blkif, char *buf)
> {
> char *devpath, *devname;
> @@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> init_completion(&blkif->drain_complete);
> atomic_set(&blkif->drain, 0);
> blkif->st_print = jiffies;
> - init_waitqueue_head(&blkif->waiting_to_free);
> blkif->persistent_gnts.rb_node = NULL;
> spin_lock_init(&blkif->free_pages_lock);
> INIT_LIST_HEAD(&blkif->free_pages);
> @@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>
> INIT_LIST_HEAD(&blkif->pending_free);
> + INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>
> for (i = 0; i < XEN_BLKIF_REQS; i++) {
> req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
> return 0;
> }
>
> -static void xen_blkif_disconnect(struct xen_blkif *blkif)
> +static int xen_blkif_disconnect(struct xen_blkif *blkif)
> {
> if (blkif->xenblkd) {
> kthread_stop(blkif->xenblkd);
> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
> blkif->xenblkd = NULL;
> }
>
> - atomic_dec(&blkif->refcnt);
> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
> - atomic_inc(&blkif->refcnt);
> + /* The above kthread_stop() guarantees that at this point we
> + * don't have any discard_io or other_io requests. So, checking
> + * for inflight IO is enough.
> + */
> + if (!atomic_read(&blkif->inflight)) {
> + if (blkif->irq) {
> + unbind_from_irqhandler(blkif->irq, blkif);
> + blkif->irq = 0;
> + }
>
> - if (blkif->irq) {
> - unbind_from_irqhandler(blkif->irq, blkif);
> - blkif->irq = 0;
> - }
> + if (blkif->blk_rings.common.sring) {
> + xenbus_unmap_ring_vfree(blkif->be->dev,
> + blkif->blk_ring);
> + blkif->blk_rings.common.sring = NULL;
> + }
>
> - if (blkif->blk_rings.common.sring) {
> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
> - blkif->blk_rings.common.sring = NULL;
> + return 0;
> }
> +
> + return -EBUSY;
> }
>
> static void xen_blkif_free(struct xen_blkif *blkif)
> @@ -259,8 +280,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> struct pending_req *req, *n;
> int i = 0, j;
>
> - if (!atomic_dec_and_test(&blkif->refcnt))
> - BUG();
> + xen_blkif_disconnect(blkif);
> + xen_vbd_free(&blkif->vbd);
>
> /* Remove all persistent grants and the cache of ballooned pages. */
> xen_blkbk_free_caches(blkif);
> @@ -449,16 +470,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
> be->backend_watch.node = NULL;
> }
>
> + dev_set_drvdata(&dev->dev, NULL);
> +
> if (be->blkif) {
> xen_blkif_disconnect(be->blkif);
> - xen_vbd_free(&be->blkif->vbd);
> - xen_blkif_free(be->blkif);
> - be->blkif = NULL;
> + xen_blkif_put(be->blkif);
> }
>
> kfree(be->mode);
> kfree(be);
> - dev_set_drvdata(&dev->dev, NULL);
> return 0;
> }
>
> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
> * Enforce precondition before potential leak point.
> * xen_blkif_disconnect() is idempotent.
> */
> - xen_blkif_disconnect(be->blkif);
> + err = xen_blkif_disconnect(be->blkif);
> + if (err)
> + break;
>
> err = connect_ring(be);
> if (err)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-09 23:56 ` Matt Wilson
@ 2014-05-12 6:42 ` Vasiliy Tolstov
2014-05-12 8:59 ` Valentin Priescu
0 siblings, 1 reply; 27+ messages in thread
From: Vasiliy Tolstov @ 2014-05-12 6:42 UTC (permalink / raw)
To: Matt Wilson; +Cc: xen-devel, Matt Wilson, Valentin Priescu, Valentin Priescu
As i understand this is needed for stable too?
2014-05-10 3:56 GMT+04:00 Matt Wilson <msw@linux.com>:
> On Sat, May 10, 2014 at 01:34:39AM +0200, Valentin Priescu wrote:
>> From: Valentin Priescu <priescuv@amazon.com>
>>
>> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
>> requests to finish. If the VBD is attached to a hot-swappable disk, then
>> xenwatch can hang for a long period of time, stalling other watches.
>>
>> INFO: task xenwatch:39 blocked for more than 120 seconds.
>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
>> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
>> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
>> Call Trace:
>> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
>> [<ffffffff8128f761>] ? list_del+0x11/0x40
>> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
>> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
>> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
>> [<ffffffff8147a26a>] schedule+0x3a/0x60
>> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
>> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
>> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
>> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
>> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
>> [<ffffffff81346488>] device_release_driver+0x28/0x40
>> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
>> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
>> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
>> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
>> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
>> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
>> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
>> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
>> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
>> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
>> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
>> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
>> [<ffffffff810799d6>] kthread+0x96/0xa0
>> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
>> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
>> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
>> [<ffffffff81485930>] ? gs_change+0x13/0x13
>>
>> With this patch, when there is still pending I/O, the actual disconnect
>> is done by the last reference holder (last pending I/O request). In this
>> case, xenwatch doesn't block indefinitely.
>>
>> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
>> Reviewed-by: Steven Kady <stevkady@amazon.com>
>> Reviewed-by: Steven Noonan <snoonan@amazon.com>
>> Cc: Matt Wilson <msw@amazon.com>
>
> Acked-by: Matt Wilson <msw@amazon.com>
>
>> ---
>> drivers/block/xen-blkback/common.h | 4 +--
>> drivers/block/xen-blkback/xenbus.c | 60 ++++++++++++++++++++++++++------------
>> 2 files changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
>> index be05277..f65b807 100644
>> --- a/drivers/block/xen-blkback/common.h
>> +++ b/drivers/block/xen-blkback/common.h
>> @@ -314,7 +314,7 @@ struct xen_blkif {
>> unsigned long long st_rd_sect;
>> unsigned long long st_wr_sect;
>>
>> - wait_queue_head_t waiting_to_free;
>> + struct work_struct free_work;
>> /* Thread shutdown wait queue. */
>> wait_queue_head_t shutdown_wq;
>> };
>> @@ -361,7 +361,7 @@ struct pending_req {
>> #define xen_blkif_put(_b) \
>> do { \
>> if (atomic_dec_and_test(&(_b)->refcnt)) \
>> - wake_up(&(_b)->waiting_to_free);\
>> + schedule_work(&(_b)->free_work);\
>> } while (0)
>>
>> struct phys_req {
>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>> index 9a547e6..7b6124e 100644
>> --- a/drivers/block/xen-blkback/xenbus.c
>> +++ b/drivers/block/xen-blkback/xenbus.c
>> @@ -35,12 +35,26 @@ static void connect(struct backend_info *);
>> static int connect_ring(struct backend_info *);
>> static void backend_changed(struct xenbus_watch *, const char **,
>> unsigned int);
>> +static void xen_blkif_free(struct xen_blkif *blkif);
>> +static void xen_vbd_free(struct xen_vbd *vbd);
>>
>> struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
>> {
>> return be->dev;
>> }
>>
>> +/*
>> + * The last request could free the device from softirq context and
>> + * xen_blkif_free() can sleep.
>> + */
>> +static void xen_blkif_deferred_free(struct work_struct *work)
>> +{
>> + struct xen_blkif *blkif;
>> +
>> + blkif = container_of(work, struct xen_blkif, free_work);
>> + xen_blkif_free(blkif);
>> +}
>> +
>> static int blkback_name(struct xen_blkif *blkif, char *buf)
>> {
>> char *devpath, *devname;
>> @@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>> init_completion(&blkif->drain_complete);
>> atomic_set(&blkif->drain, 0);
>> blkif->st_print = jiffies;
>> - init_waitqueue_head(&blkif->waiting_to_free);
>> blkif->persistent_gnts.rb_node = NULL;
>> spin_lock_init(&blkif->free_pages_lock);
>> INIT_LIST_HEAD(&blkif->free_pages);
>> @@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>>
>> INIT_LIST_HEAD(&blkif->pending_free);
>> + INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>>
>> for (i = 0; i < XEN_BLKIF_REQS; i++) {
>> req = kzalloc(sizeof(*req), GFP_KERNEL);
>> @@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
>> return 0;
>> }
>>
>> -static void xen_blkif_disconnect(struct xen_blkif *blkif)
>> +static int xen_blkif_disconnect(struct xen_blkif *blkif)
>> {
>> if (blkif->xenblkd) {
>> kthread_stop(blkif->xenblkd);
>> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
>> blkif->xenblkd = NULL;
>> }
>>
>> - atomic_dec(&blkif->refcnt);
>> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
>> - atomic_inc(&blkif->refcnt);
>> + /* The above kthread_stop() guarantees that at this point we
>> + * don't have any discard_io or other_io requests. So, checking
>> + * for inflight IO is enough.
>> + */
>> + if (!atomic_read(&blkif->inflight)) {
>> + if (blkif->irq) {
>> + unbind_from_irqhandler(blkif->irq, blkif);
>> + blkif->irq = 0;
>> + }
>>
>> - if (blkif->irq) {
>> - unbind_from_irqhandler(blkif->irq, blkif);
>> - blkif->irq = 0;
>> - }
>> + if (blkif->blk_rings.common.sring) {
>> + xenbus_unmap_ring_vfree(blkif->be->dev,
>> + blkif->blk_ring);
>> + blkif->blk_rings.common.sring = NULL;
>> + }
>>
>> - if (blkif->blk_rings.common.sring) {
>> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
>> - blkif->blk_rings.common.sring = NULL;
>> + return 0;
>> }
>> +
>> + return -EBUSY;
>> }
>>
>> static void xen_blkif_free(struct xen_blkif *blkif)
>> @@ -259,8 +280,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>> struct pending_req *req, *n;
>> int i = 0, j;
>>
>> - if (!atomic_dec_and_test(&blkif->refcnt))
>> - BUG();
>> + xen_blkif_disconnect(blkif);
>> + xen_vbd_free(&blkif->vbd);
>>
>> /* Remove all persistent grants and the cache of ballooned pages. */
>> xen_blkbk_free_caches(blkif);
>> @@ -449,16 +470,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
>> be->backend_watch.node = NULL;
>> }
>>
>> + dev_set_drvdata(&dev->dev, NULL);
>> +
>> if (be->blkif) {
>> xen_blkif_disconnect(be->blkif);
>> - xen_vbd_free(&be->blkif->vbd);
>> - xen_blkif_free(be->blkif);
>> - be->blkif = NULL;
>> + xen_blkif_put(be->blkif);
>> }
>>
>> kfree(be->mode);
>> kfree(be);
>> - dev_set_drvdata(&dev->dev, NULL);
>> return 0;
>> }
>>
>> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
>> * Enforce precondition before potential leak point.
>> * xen_blkif_disconnect() is idempotent.
>> */
>> - xen_blkif_disconnect(be->blkif);
>> + err = xen_blkif_disconnect(be->blkif);
>> + if (err)
>> + break;
>>
>> err = connect_ring(be);
>> if (err)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
--
Vasiliy Tolstov,
e-mail: v.tolstov@selfip.ru
jabber: vase@selfip.ru
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 6:42 ` Vasiliy Tolstov
@ 2014-05-12 8:59 ` Valentin Priescu
2014-05-12 9:12 ` Alexey
0 siblings, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 8:59 UTC (permalink / raw)
To: Vasiliy Tolstov, Matt Wilson; +Cc: xen-devel, Matt Wilson, Valentin Priescu
Yes, this patch applies to all kernels >= v3.14 (including the latest
stable).
On 05/12/2014 08:42 AM, Vasiliy Tolstov wrote:
> As i understand this is needed for stable too?
>
> 2014-05-10 3:56 GMT+04:00 Matt Wilson <msw@linux.com>:
>> On Sat, May 10, 2014 at 01:34:39AM +0200, Valentin Priescu wrote:
>>> From: Valentin Priescu <priescuv@amazon.com>
>>>
>>> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
>>> requests to finish. If the VBD is attached to a hot-swappable disk, then
>>> xenwatch can hang for a long period of time, stalling other watches.
>>>
>>> INFO: task xenwatch:39 blocked for more than 120 seconds.
>>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
>>> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
>>> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
>>> Call Trace:
>>> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
>>> [<ffffffff8128f761>] ? list_del+0x11/0x40
>>> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
>>> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
>>> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
>>> [<ffffffff8147a26a>] schedule+0x3a/0x60
>>> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
>>> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
>>> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
>>> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
>>> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
>>> [<ffffffff81346488>] device_release_driver+0x28/0x40
>>> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
>>> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
>>> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
>>> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
>>> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
>>> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
>>> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
>>> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
>>> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
>>> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
>>> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
>>> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
>>> [<ffffffff810799d6>] kthread+0x96/0xa0
>>> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
>>> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
>>> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
>>> [<ffffffff81485930>] ? gs_change+0x13/0x13
>>>
>>> With this patch, when there is still pending I/O, the actual disconnect
>>> is done by the last reference holder (last pending I/O request). In this
>>> case, xenwatch doesn't block indefinitely.
>>>
>>> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
>>> Reviewed-by: Steven Kady <stevkady@amazon.com>
>>> Reviewed-by: Steven Noonan <snoonan@amazon.com>
>>> Cc: Matt Wilson <msw@amazon.com>
>>
>> Acked-by: Matt Wilson <msw@amazon.com>
>>
>>> ---
>>> drivers/block/xen-blkback/common.h | 4 +--
>>> drivers/block/xen-blkback/xenbus.c | 60 ++++++++++++++++++++++++++------------
>>> 2 files changed, 43 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
>>> index be05277..f65b807 100644
>>> --- a/drivers/block/xen-blkback/common.h
>>> +++ b/drivers/block/xen-blkback/common.h
>>> @@ -314,7 +314,7 @@ struct xen_blkif {
>>> unsigned long long st_rd_sect;
>>> unsigned long long st_wr_sect;
>>>
>>> - wait_queue_head_t waiting_to_free;
>>> + struct work_struct free_work;
>>> /* Thread shutdown wait queue. */
>>> wait_queue_head_t shutdown_wq;
>>> };
>>> @@ -361,7 +361,7 @@ struct pending_req {
>>> #define xen_blkif_put(_b) \
>>> do { \
>>> if (atomic_dec_and_test(&(_b)->refcnt)) \
>>> - wake_up(&(_b)->waiting_to_free);\
>>> + schedule_work(&(_b)->free_work);\
>>> } while (0)
>>>
>>> struct phys_req {
>>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>>> index 9a547e6..7b6124e 100644
>>> --- a/drivers/block/xen-blkback/xenbus.c
>>> +++ b/drivers/block/xen-blkback/xenbus.c
>>> @@ -35,12 +35,26 @@ static void connect(struct backend_info *);
>>> static int connect_ring(struct backend_info *);
>>> static void backend_changed(struct xenbus_watch *, const char **,
>>> unsigned int);
>>> +static void xen_blkif_free(struct xen_blkif *blkif);
>>> +static void xen_vbd_free(struct xen_vbd *vbd);
>>>
>>> struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
>>> {
>>> return be->dev;
>>> }
>>>
>>> +/*
>>> + * The last request could free the device from softirq context and
>>> + * xen_blkif_free() can sleep.
>>> + */
>>> +static void xen_blkif_deferred_free(struct work_struct *work)
>>> +{
>>> + struct xen_blkif *blkif;
>>> +
>>> + blkif = container_of(work, struct xen_blkif, free_work);
>>> + xen_blkif_free(blkif);
>>> +}
>>> +
>>> static int blkback_name(struct xen_blkif *blkif, char *buf)
>>> {
>>> char *devpath, *devname;
>>> @@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>>> init_completion(&blkif->drain_complete);
>>> atomic_set(&blkif->drain, 0);
>>> blkif->st_print = jiffies;
>>> - init_waitqueue_head(&blkif->waiting_to_free);
>>> blkif->persistent_gnts.rb_node = NULL;
>>> spin_lock_init(&blkif->free_pages_lock);
>>> INIT_LIST_HEAD(&blkif->free_pages);
>>> @@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>>> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>>>
>>> INIT_LIST_HEAD(&blkif->pending_free);
>>> + INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>>>
>>> for (i = 0; i < XEN_BLKIF_REQS; i++) {
>>> req = kzalloc(sizeof(*req), GFP_KERNEL);
>>> @@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
>>> return 0;
>>> }
>>>
>>> -static void xen_blkif_disconnect(struct xen_blkif *blkif)
>>> +static int xen_blkif_disconnect(struct xen_blkif *blkif)
>>> {
>>> if (blkif->xenblkd) {
>>> kthread_stop(blkif->xenblkd);
>>> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
>>> blkif->xenblkd = NULL;
>>> }
>>>
>>> - atomic_dec(&blkif->refcnt);
>>> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
>>> - atomic_inc(&blkif->refcnt);
>>> + /* The above kthread_stop() guarantees that at this point we
>>> + * don't have any discard_io or other_io requests. So, checking
>>> + * for inflight IO is enough.
>>> + */
>>> + if (!atomic_read(&blkif->inflight)) {
>>> + if (blkif->irq) {
>>> + unbind_from_irqhandler(blkif->irq, blkif);
>>> + blkif->irq = 0;
>>> + }
>>>
>>> - if (blkif->irq) {
>>> - unbind_from_irqhandler(blkif->irq, blkif);
>>> - blkif->irq = 0;
>>> - }
>>> + if (blkif->blk_rings.common.sring) {
>>> + xenbus_unmap_ring_vfree(blkif->be->dev,
>>> + blkif->blk_ring);
>>> + blkif->blk_rings.common.sring = NULL;
>>> + }
>>>
>>> - if (blkif->blk_rings.common.sring) {
>>> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
>>> - blkif->blk_rings.common.sring = NULL;
>>> + return 0;
>>> }
>>> +
>>> + return -EBUSY;
>>> }
>>>
>>> static void xen_blkif_free(struct xen_blkif *blkif)
>>> @@ -259,8 +280,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>>> struct pending_req *req, *n;
>>> int i = 0, j;
>>>
>>> - if (!atomic_dec_and_test(&blkif->refcnt))
>>> - BUG();
>>> + xen_blkif_disconnect(blkif);
>>> + xen_vbd_free(&blkif->vbd);
>>>
>>> /* Remove all persistent grants and the cache of ballooned pages. */
>>> xen_blkbk_free_caches(blkif);
>>> @@ -449,16 +470,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
>>> be->backend_watch.node = NULL;
>>> }
>>>
>>> + dev_set_drvdata(&dev->dev, NULL);
>>> +
>>> if (be->blkif) {
>>> xen_blkif_disconnect(be->blkif);
>>> - xen_vbd_free(&be->blkif->vbd);
>>> - xen_blkif_free(be->blkif);
>>> - be->blkif = NULL;
>>> + xen_blkif_put(be->blkif);
>>> }
>>>
>>> kfree(be->mode);
>>> kfree(be);
>>> - dev_set_drvdata(&dev->dev, NULL);
>>> return 0;
>>> }
>>>
>>> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
>>> * Enforce precondition before potential leak point.
>>> * xen_blkif_disconnect() is idempotent.
>>> */
>>> - xen_blkif_disconnect(be->blkif);
>>> + err = xen_blkif_disconnect(be->blkif);
>>> + if (err)
>>> + break;
>>>
>>> err = connect_ring(be);
>>> if (err)
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 8:59 ` Valentin Priescu
@ 2014-05-12 9:12 ` Alexey
2014-05-12 10:50 ` Valentin Priescu
0 siblings, 1 reply; 27+ messages in thread
From: Alexey @ 2014-05-12 9:12 UTC (permalink / raw)
To: xen-devel
And what about 3.10 kernel?
On 2014-05-12 12:59, Valentin Priescu wrote:
> Yes, this patch applies to all kernels >= v3.14 (including the latest
> stable).
>
> On 05/12/2014 08:42 AM, Vasiliy Tolstov wrote:
>> As i understand this is needed for stable too?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-09 23:34 [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch Valentin Priescu
2014-05-09 23:56 ` Matt Wilson
@ 2014-05-12 9:39 ` David Vrabel
2014-05-12 10:41 ` Valentin Priescu
2014-05-12 16:40 ` Boris Ostrovsky
2014-05-12 19:25 ` [PATCH v2] " Valentin Priescu
3 siblings, 1 reply; 27+ messages in thread
From: David Vrabel @ 2014-05-12 9:39 UTC (permalink / raw)
To: Valentin Priescu, xen-devel; +Cc: Matt Wilson, Valentin Priescu
On 10/05/14 00:34, Valentin Priescu wrote:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
[...]
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
[...]
> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
> blkif->xenblkd = NULL;
> }
>
> - atomic_dec(&blkif->refcnt);
> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
> - atomic_inc(&blkif->refcnt);
> + /* The above kthread_stop() guarantees that at this point we
> + * don't have any discard_io or other_io requests. So, checking
> + * for inflight IO is enough.
> + */
> + if (!atomic_read(&blkif->inflight)) {
if (atomic_read(&blkif->inflight > 0)
return -EBUSY;
> + if (blkif->irq) {
> + unbind_from_irqhandler(blkif->irq, blkif);
> + blkif->irq = 0;
> + }
>
> - if (blkif->irq) {
> - unbind_from_irqhandler(blkif->irq, blkif);
> - blkif->irq = 0;
> - }
> + if (blkif->blk_rings.common.sring) {
> + xenbus_unmap_ring_vfree(blkif->be->dev,
> + blkif->blk_ring);
> + blkif->blk_rings.common.sring = NULL;
> + }
>
> - if (blkif->blk_rings.common.sring) {
> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
> - blkif->blk_rings.common.sring = NULL;
> + return 0;
> }
> +
> + return -EBUSY;
> }
[...]
> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
> * Enforce precondition before potential leak point.
> * xen_blkif_disconnect() is idempotent.
> */
> - xen_blkif_disconnect(be->blkif);
> + err = xen_blkif_disconnect(be->blkif);
> + if (err)
> + break;
>
> err = connect_ring(be);
> if (err)
If xen_blkif_disconnect() returns -EBUSY, nothing will trigger the
reconnect.
David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 9:39 ` David Vrabel
@ 2014-05-12 10:41 ` Valentin Priescu
2014-05-12 10:43 ` David Vrabel
0 siblings, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 10:41 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Matt Wilson, Valentin Priescu
On Mon, May 12, 2014 at 11:39 AM, David Vrabel <david.vrabel@citrix.com> wrote:
> On 10/05/14 00:34, Valentin Priescu wrote:
>> From: Valentin Priescu <priescuv@amazon.com>
>> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
>> blkif->xenblkd = NULL;
>> }
>>
>> - atomic_dec(&blkif->refcnt);
>> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
>> - atomic_inc(&blkif->refcnt);
>> + /* The above kthread_stop() guarantees that at this point we
>> + * don't have any discard_io or other_io requests. So, checking
>> + * for inflight IO is enough.
>> + */
>> + if (!atomic_read(&blkif->inflight)) {
>
> if (atomic_read(&blkif->inflight > 0)
> return -EBUSY;
>
Will do.
>> + if (blkif->irq) {
>> + unbind_from_irqhandler(blkif->irq, blkif);
>> + blkif->irq = 0;
>> + }
>>
>> - if (blkif->irq) {
>> - unbind_from_irqhandler(blkif->irq, blkif);
>> - blkif->irq = 0;
>> - }
>> + if (blkif->blk_rings.common.sring) {
>> + xenbus_unmap_ring_vfree(blkif->be->dev,
>> + blkif->blk_ring);
>> + blkif->blk_rings.common.sring = NULL;
>> + }
>>
>> - if (blkif->blk_rings.common.sring) {
>> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
>> - blkif->blk_rings.common.sring = NULL;
>> + return 0;
>> }
>> +
>> + return -EBUSY;
>> }
> [...]
>> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
>> * Enforce precondition before potential leak point.
>> * xen_blkif_disconnect() is idempotent.
>> */
>> - xen_blkif_disconnect(be->blkif);
>> + err = xen_blkif_disconnect(be->blkif);
>> + if (err)
>> + break;
>>
>> err = connect_ring(be);
>> if (err)
>
> If xen_blkif_disconnect() returns -EBUSY, nothing will trigger the
> reconnect.
>
Yeah, I should do something like:
err = xen_blkif_disconnect(be->blkif);
if (err) {
xenbus_dev_fatal(dev, err, "disconnecting %s",
dev->otherend);
break;
}
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 10:41 ` Valentin Priescu
@ 2014-05-12 10:43 ` David Vrabel
2014-05-12 10:47 ` Valentin Priescu
0 siblings, 1 reply; 27+ messages in thread
From: David Vrabel @ 2014-05-12 10:43 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, Matt Wilson, Valentin Priescu
On 12/05/14 11:41, Valentin Priescu wrote:
> On Mon, May 12, 2014 at 11:39 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>> If xen_blkif_disconnect() returns -EBUSY, nothing will trigger the
>> reconnect.
>>
>
> Yeah, I should do something like:
> err = xen_blkif_disconnect(be->blkif);
> if (err) {
> xenbus_dev_fatal(dev, err, "disconnecting %s",
> dev->otherend);
I think that's ok, but the error message should be "pending I/O". You
don't need to include the name of the otherend in the error message.
David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 10:43 ` David Vrabel
@ 2014-05-12 10:47 ` Valentin Priescu
0 siblings, 0 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 10:47 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Matt Wilson, Valentin Priescu
On Mon, May 12, 2014 at 12:43 PM, David Vrabel <david.vrabel@citrix.com> wrote:
>>
>> Yeah, I should do something like:
>> err = xen_blkif_disconnect(be->blkif);
>> if (err) {
>> xenbus_dev_fatal(dev, err, "disconnecting %s",
>> dev->otherend);
>
> I think that's ok, but the error message should be "pending I/O". You
> don't need to include the name of the otherend in the error message.
>
> David
Agree.
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 9:12 ` Alexey
@ 2014-05-12 10:50 ` Valentin Priescu
0 siblings, 0 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 10:50 UTC (permalink / raw)
To: Alexey; +Cc: xen-devel
On Mon, May 12, 2014 at 11:12 AM, Alexey <alukardd@alukardd.org> wrote:
> And what about 3.10 kernel?
>
This patch depends on commit
c05f3e3c85df1d89673e00cee7ece5ae4eb4c6ec xen-blkback: fix shutdown race
introduced in 3.14.
>
> On 2014-05-12 12:59, Valentin Priescu wrote:
>>
>> Yes, this patch applies to all kernels >= v3.14 (including the latest
>> stable).
>>
>> On 05/12/2014 08:42 AM, Vasiliy Tolstov wrote:
>>>
>>> As i understand this is needed for stable too?
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-09 23:34 [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch Valentin Priescu
2014-05-09 23:56 ` Matt Wilson
2014-05-12 9:39 ` David Vrabel
@ 2014-05-12 16:40 ` Boris Ostrovsky
2014-05-12 17:04 ` Valentin Priescu
2014-05-12 19:25 ` [PATCH v2] " Valentin Priescu
3 siblings, 1 reply; 27+ messages in thread
From: Boris Ostrovsky @ 2014-05-12 16:40 UTC (permalink / raw)
To: Valentin Priescu
Cc: Tülin İzer, xen-devel, Matt Wilson, Valentin Priescu
On 05/09/2014 07:34 PM, Valentin Priescu wrote:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
Note that we have a GSoC student (copied here) who is going to be
looking at xenwatch's single-threadness (?) over the summer.
-boris
>
> INFO: task xenwatch:39 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
> Call Trace:
> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
> [<ffffffff8128f761>] ? list_del+0x11/0x40
> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
> [<ffffffff8147a26a>] schedule+0x3a/0x60
> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
> [<ffffffff81346488>] device_release_driver+0x28/0x40
> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
> [<ffffffff810799d6>] kthread+0x96/0xa0
> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
> [<ffffffff81485930>] ? gs_change+0x13/0x13
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
>
> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> Reviewed-by: Steven Kady <stevkady@amazon.com>
> Reviewed-by: Steven Noonan <snoonan@amazon.com>
> Cc: Matt Wilson <msw@amazon.com>
> ---
> drivers/block/xen-blkback/common.h | 4 +--
> drivers/block/xen-blkback/xenbus.c | 60 ++++++++++++++++++++++++++------------
> 2 files changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
> index be05277..f65b807 100644
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -314,7 +314,7 @@ struct xen_blkif {
> unsigned long long st_rd_sect;
> unsigned long long st_wr_sect;
>
> - wait_queue_head_t waiting_to_free;
> + struct work_struct free_work;
> /* Thread shutdown wait queue. */
> wait_queue_head_t shutdown_wq;
> };
> @@ -361,7 +361,7 @@ struct pending_req {
> #define xen_blkif_put(_b) \
> do { \
> if (atomic_dec_and_test(&(_b)->refcnt)) \
> - wake_up(&(_b)->waiting_to_free);\
> + schedule_work(&(_b)->free_work);\
> } while (0)
>
> struct phys_req {
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 9a547e6..7b6124e 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -35,12 +35,26 @@ static void connect(struct backend_info *);
> static int connect_ring(struct backend_info *);
> static void backend_changed(struct xenbus_watch *, const char **,
> unsigned int);
> +static void xen_blkif_free(struct xen_blkif *blkif);
> +static void xen_vbd_free(struct xen_vbd *vbd);
>
> struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
> {
> return be->dev;
> }
>
> +/*
> + * The last request could free the device from softirq context and
> + * xen_blkif_free() can sleep.
> + */
> +static void xen_blkif_deferred_free(struct work_struct *work)
> +{
> + struct xen_blkif *blkif;
> +
> + blkif = container_of(work, struct xen_blkif, free_work);
> + xen_blkif_free(blkif);
> +}
> +
> static int blkback_name(struct xen_blkif *blkif, char *buf)
> {
> char *devpath, *devname;
> @@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> init_completion(&blkif->drain_complete);
> atomic_set(&blkif->drain, 0);
> blkif->st_print = jiffies;
> - init_waitqueue_head(&blkif->waiting_to_free);
> blkif->persistent_gnts.rb_node = NULL;
> spin_lock_init(&blkif->free_pages_lock);
> INIT_LIST_HEAD(&blkif->free_pages);
> @@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>
> INIT_LIST_HEAD(&blkif->pending_free);
> + INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>
> for (i = 0; i < XEN_BLKIF_REQS; i++) {
> req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
> return 0;
> }
>
> -static void xen_blkif_disconnect(struct xen_blkif *blkif)
> +static int xen_blkif_disconnect(struct xen_blkif *blkif)
> {
> if (blkif->xenblkd) {
> kthread_stop(blkif->xenblkd);
> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
> blkif->xenblkd = NULL;
> }
>
> - atomic_dec(&blkif->refcnt);
> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
> - atomic_inc(&blkif->refcnt);
> + /* The above kthread_stop() guarantees that at this point we
> + * don't have any discard_io or other_io requests. So, checking
> + * for inflight IO is enough.
> + */
> + if (!atomic_read(&blkif->inflight)) {
> + if (blkif->irq) {
> + unbind_from_irqhandler(blkif->irq, blkif);
> + blkif->irq = 0;
> + }
>
> - if (blkif->irq) {
> - unbind_from_irqhandler(blkif->irq, blkif);
> - blkif->irq = 0;
> - }
> + if (blkif->blk_rings.common.sring) {
> + xenbus_unmap_ring_vfree(blkif->be->dev,
> + blkif->blk_ring);
> + blkif->blk_rings.common.sring = NULL;
> + }
>
> - if (blkif->blk_rings.common.sring) {
> - xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
> - blkif->blk_rings.common.sring = NULL;
> + return 0;
> }
> +
> + return -EBUSY;
> }
>
> static void xen_blkif_free(struct xen_blkif *blkif)
> @@ -259,8 +280,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> struct pending_req *req, *n;
> int i = 0, j;
>
> - if (!atomic_dec_and_test(&blkif->refcnt))
> - BUG();
> + xen_blkif_disconnect(blkif);
> + xen_vbd_free(&blkif->vbd);
>
> /* Remove all persistent grants and the cache of ballooned pages. */
> xen_blkbk_free_caches(blkif);
> @@ -449,16 +470,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
> be->backend_watch.node = NULL;
> }
>
> + dev_set_drvdata(&dev->dev, NULL);
> +
> if (be->blkif) {
> xen_blkif_disconnect(be->blkif);
> - xen_vbd_free(&be->blkif->vbd);
> - xen_blkif_free(be->blkif);
> - be->blkif = NULL;
> + xen_blkif_put(be->blkif);
> }
>
> kfree(be->mode);
> kfree(be);
> - dev_set_drvdata(&dev->dev, NULL);
> return 0;
> }
>
> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
> * Enforce precondition before potential leak point.
> * xen_blkif_disconnect() is idempotent.
> */
> - xen_blkif_disconnect(be->blkif);
> + err = xen_blkif_disconnect(be->blkif);
> + if (err)
> + break;
>
> err = connect_ring(be);
> if (err)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 16:40 ` Boris Ostrovsky
@ 2014-05-12 17:04 ` Valentin Priescu
2014-05-12 17:50 ` Boris Ostrovsky
0 siblings, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 17:04 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Tülin İzer, xen-devel, Matt Wilson, Valentin Priescu
On Mon, May 12, 2014 at 6:40 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> On 05/09/2014 07:34 PM, Valentin Priescu wrote:
>>
>> From: Valentin Priescu <priescuv@amazon.com>
>>
>> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
>> requests to finish. If the VBD is attached to a hot-swappable disk, then
>> xenwatch can hang for a long period of time, stalling other watches.
>
>
> Note that we have a GSoC student (copied here) who is going to be looking at
> xenwatch's single-threadness (?) over the summer.
>
AFAIK, in older kernel versions, you could pass a flag to a registered
watch that told
xenwatch to spawn a new kthread when it handles the events. That
didn't work well
and it was removed.
Anyway, whatever solution he comes up with, I think he still needs to
make similar
changes in the disconnect path for blkback. Besides, blocking
uninterruptible for a long
period of time is not OK.
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 17:04 ` Valentin Priescu
@ 2014-05-12 17:50 ` Boris Ostrovsky
0 siblings, 0 replies; 27+ messages in thread
From: Boris Ostrovsky @ 2014-05-12 17:50 UTC (permalink / raw)
To: Valentin Priescu
Cc: Tülin İzer, xen-devel, Matt Wilson, Valentin Priescu
On 05/12/2014 01:04 PM, Valentin Priescu wrote:
> On Mon, May 12, 2014 at 6:40 PM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> On 05/09/2014 07:34 PM, Valentin Priescu wrote:
>>> From: Valentin Priescu <priescuv@amazon.com>
>>>
>>> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
>>> requests to finish. If the VBD is attached to a hot-swappable disk, then
>>> xenwatch can hang for a long period of time, stalling other watches.
>>
>> Note that we have a GSoC student (copied here) who is going to be looking at
>> xenwatch's single-threadness (?) over the summer.
>>
> AFAIK, in older kernel versions, you could pass a flag to a registered
> watch that told
> xenwatch to spawn a new kthread when it handles the events. That
> didn't work well
> and it was removed.
Right, it would have to be something coarser than a per-event thread.
Maybe per-guest.
The goal is more scalability than correctness but as a side-effect it
would prevent xenwatch from being stuck for everyone.
-boris
> Anyway, whatever solution he comes up with, I think he still needs to
> make similar
> changes in the disconnect path for blkback. Besides, blocking
> uninterruptible for a long
> period of time is not OK.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-09 23:34 [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch Valentin Priescu
` (2 preceding siblings ...)
2014-05-12 16:40 ` Boris Ostrovsky
@ 2014-05-12 19:25 ` Valentin Priescu
2014-05-13 9:32 ` David Vrabel
2014-05-20 20:28 ` [PATCH v3] " Valentin Priescu
3 siblings, 2 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-12 19:25 UTC (permalink / raw)
To: xen-devel; +Cc: Valentin Priescu, David Vrabel, Matt Wilson
From: Valentin Priescu <priescuv@amazon.com>
Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
requests to finish. If the VBD is attached to a hot-swappable disk, then
xenwatch can hang for a long period of time, stalling other watches.
INFO: task xenwatch:39 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
Call Trace:
[<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
[<ffffffff8128f761>] ? list_del+0x11/0x40
[<ffffffff8147a080>] ? wait_for_common+0x60/0x160
[<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
[<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
[<ffffffff8147a26a>] schedule+0x3a/0x60
[<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
[<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
[<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
[<ffffffff81346488>] device_release_driver+0x28/0x40
[<ffffffff813456e8>] bus_remove_device+0x78/0xe0
[<ffffffff81342c9f>] device_del+0x12f/0x1a0
[<ffffffff81342d2d>] device_unregister+0x1d/0x60
[<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
[<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
[<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
[<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
[<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
[<ffffffff8130b4cb>] frontend_changed+0xb/0x10
[<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
[<ffffffff810799d6>] kthread+0x96/0xa0
[<ffffffff81485934>] kernel_thread_helper+0x4/0x10
[<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
[<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
[<ffffffff81485930>] ? gs_change+0x13/0x13
With this patch, when there is still pending I/O, the actual disconnect
is done by the last reference holder (last pending I/O request). In this
case, xenwatch doesn't block indefinitely.
Signed-off-by: Valentin Priescu <priescuv@amazon.com>
Reviewed-by: Steven Kady <stevkady@amazon.com>
Reviewed-by: Steven Noonan <snoonan@amazon.com>
Acked-by: Matt Wilson <msw@amazon.com>
---
v2: Reworked an 'if' statement in xen_blkif_disconnect(), as
suggested by David Vrabel <david.vrabel@citrix.com>
Handle the case when xen_blkif_disconnect() returns -EBUSY on
frontend_changed(), as noted by David Vrabel <david.vrabel@citrix.com>
drivers/block/xen-blkback/common.h | 4 ++--
drivers/block/xen-blkback/xenbus.c | 46 ++++++++++++++++++++++++++++----------
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index be05277..f65b807 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -314,7 +314,7 @@ struct xen_blkif {
unsigned long long st_rd_sect;
unsigned long long st_wr_sect;
- wait_queue_head_t waiting_to_free;
+ struct work_struct free_work;
/* Thread shutdown wait queue. */
wait_queue_head_t shutdown_wq;
};
@@ -361,7 +361,7 @@ struct pending_req {
#define xen_blkif_put(_b) \
do { \
if (atomic_dec_and_test(&(_b)->refcnt)) \
- wake_up(&(_b)->waiting_to_free);\
+ schedule_work(&(_b)->free_work);\
} while (0)
struct phys_req {
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 9a547e6..e911d28 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -35,12 +35,26 @@ static void connect(struct backend_info *);
static int connect_ring(struct backend_info *);
static void backend_changed(struct xenbus_watch *, const char **,
unsigned int);
+static void xen_blkif_free(struct xen_blkif *blkif);
+static void xen_vbd_free(struct xen_vbd *vbd);
struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
{
return be->dev;
}
+/*
+ * The last request could free the device from softirq context and
+ * xen_blkif_free() can sleep.
+ */
+static void xen_blkif_deferred_free(struct work_struct *work)
+{
+ struct xen_blkif *blkif;
+
+ blkif = container_of(work, struct xen_blkif, free_work);
+ xen_blkif_free(blkif);
+}
+
static int blkback_name(struct xen_blkif *blkif, char *buf)
{
char *devpath, *devname;
@@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
init_completion(&blkif->drain_complete);
atomic_set(&blkif->drain, 0);
blkif->st_print = jiffies;
- init_waitqueue_head(&blkif->waiting_to_free);
blkif->persistent_gnts.rb_node = NULL;
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
@@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
INIT_LIST_HEAD(&blkif->pending_free);
+ INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
for (i = 0; i < XEN_BLKIF_REQS; i++) {
req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
return 0;
}
-static void xen_blkif_disconnect(struct xen_blkif *blkif)
+static int xen_blkif_disconnect(struct xen_blkif *blkif)
{
if (blkif->xenblkd) {
kthread_stop(blkif->xenblkd);
@@ -239,9 +253,12 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
blkif->xenblkd = NULL;
}
- atomic_dec(&blkif->refcnt);
- wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
- atomic_inc(&blkif->refcnt);
+ /* The above kthread_stop() guarantees that at this point we
+ * don't have any discard_io or other_io requests. So, checking
+ * for inflight IO is enough.
+ */
+ if (atomic_read(&blkif->inflight) > 0)
+ return -EBUSY;
if (blkif->irq) {
unbind_from_irqhandler(blkif->irq, blkif);
@@ -252,6 +269,8 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
blkif->blk_rings.common.sring = NULL;
}
+
+ return 0;
}
static void xen_blkif_free(struct xen_blkif *blkif)
@@ -259,8 +278,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
struct pending_req *req, *n;
int i = 0, j;
- if (!atomic_dec_and_test(&blkif->refcnt))
- BUG();
+ xen_blkif_disconnect(blkif);
+ xen_vbd_free(&blkif->vbd);
/* Remove all persistent grants and the cache of ballooned pages. */
xen_blkbk_free_caches(blkif);
@@ -449,16 +468,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
be->backend_watch.node = NULL;
}
+ dev_set_drvdata(&dev->dev, NULL);
+
if (be->blkif) {
xen_blkif_disconnect(be->blkif);
- xen_vbd_free(&be->blkif->vbd);
- xen_blkif_free(be->blkif);
- be->blkif = NULL;
+ xen_blkif_put(be->blkif);
}
kfree(be->mode);
kfree(be);
- dev_set_drvdata(&dev->dev, NULL);
return 0;
}
@@ -700,7 +718,11 @@ static void frontend_changed(struct xenbus_device *dev,
* Enforce precondition before potential leak point.
* xen_blkif_disconnect() is idempotent.
*/
- xen_blkif_disconnect(be->blkif);
+ err = xen_blkif_disconnect(be->blkif);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "pending I/O");
+ break;
+ }
err = connect_ring(be);
if (err)
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 19:25 ` [PATCH v2] " Valentin Priescu
@ 2014-05-13 9:32 ` David Vrabel
2014-05-13 17:00 ` Vasiliy Tolstov
` (2 more replies)
2014-05-20 20:28 ` [PATCH v3] " Valentin Priescu
1 sibling, 3 replies; 27+ messages in thread
From: David Vrabel @ 2014-05-13 9:32 UTC (permalink / raw)
To: Valentin Priescu, xen-devel; +Cc: Valentin Priescu, Matt Wilson
On 12/05/14 20:25, Valentin Priescu wrote:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
[...]
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> Reviewed-by: Steven Kady <stevkady@amazon.com>
> Reviewed-by: Steven Noonan <snoonan@amazon.com>
> Acked-by: Matt Wilson <msw@amazon.com>
Not sure Acked-by by a non-maintainer makes much sense. Matt, did you
mean Reviewed-by here instead?
David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-13 9:32 ` David Vrabel
@ 2014-05-13 17:00 ` Vasiliy Tolstov
2014-05-13 17:32 ` Valentin Priescu
2014-05-16 8:37 ` Valentin Priescu
2014-06-02 16:20 ` Matt Wilson
2 siblings, 1 reply; 27+ messages in thread
From: Vasiliy Tolstov @ 2014-05-13 17:00 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Matt Wilson, Valentin Priescu, Valentin Priescu
Sorry, but now i'm not understand - what patches are missed from stable 3.10.x ?
For example https://groups.google.com/forum/#!msg/linux.kernel/Ih9XKvJNGjI/RZ4huN4K4AQJ
does it included in 3.10.x?
2014-05-13 13:32 GMT+04:00 David Vrabel <david.vrabel@citrix.com>:
> On 12/05/14 20:25, Valentin Priescu wrote:
>> From: Valentin Priescu <priescuv@amazon.com>
>>
>> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
>> requests to finish. If the VBD is attached to a hot-swappable disk, then
>> xenwatch can hang for a long period of time, stalling other watches.
>>
> [...]
>>
>> With this patch, when there is still pending I/O, the actual disconnect
>> is done by the last reference holder (last pending I/O request). In this
>> case, xenwatch doesn't block indefinitely.
>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
>
>> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
>> Reviewed-by: Steven Kady <stevkady@amazon.com>
>> Reviewed-by: Steven Noonan <snoonan@amazon.com>
>> Acked-by: Matt Wilson <msw@amazon.com>
>
> Not sure Acked-by by a non-maintainer makes much sense. Matt, did you
> mean Reviewed-by here instead?
>
> David
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
--
Vasiliy Tolstov,
e-mail: v.tolstov@selfip.ru
jabber: vase@selfip.ru
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-13 17:00 ` Vasiliy Tolstov
@ 2014-05-13 17:32 ` Valentin Priescu
2014-05-13 17:39 ` Vasiliy Tolstov
0 siblings, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-05-13 17:32 UTC (permalink / raw)
To: Vasiliy Tolstov; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
On Tue, May 13, 2014 at 7:00 PM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote:
> Sorry, but now i'm not understand - what patches are missed from stable 3.10.x ?
> For example https://groups.google.com/forum/#!msg/linux.kernel/Ih9XKvJNGjI/RZ4huN4K4AQJ
> does it included in 3.10.x?
>
I'm talking about this patch:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c05f3e3c85df1d89673e00cee7ece5ae4eb4c6ec
The new variable (inflight) introduced there is used in my patch.
I don't see that patch in stable 3.10.40.
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-13 17:32 ` Valentin Priescu
@ 2014-05-13 17:39 ` Vasiliy Tolstov
0 siblings, 0 replies; 27+ messages in thread
From: Vasiliy Tolstov @ 2014-05-13 17:39 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
2014-05-13 21:32 GMT+04:00 Valentin Priescu <vali.priescu@gmail.com>:
> I'm talking about this patch:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c05f3e3c85df1d89673e00cee7ece5ae4eb4c6ec
> The new variable (inflight) introduced there is used in my patch.
> I don't see that patch in stable 3.10.40.
Thanks!
--
Vasiliy Tolstov,
e-mail: v.tolstov@selfip.ru
jabber: vase@selfip.ru
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-13 9:32 ` David Vrabel
2014-05-13 17:00 ` Vasiliy Tolstov
@ 2014-05-16 8:37 ` Valentin Priescu
2014-06-02 16:20 ` Matt Wilson
2 siblings, 0 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-16 8:37 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Valentin Priescu, Matt Wilson
On Tue, May 13, 2014 at 11:32 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
>
>> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
>> Reviewed-by: Steven Kady <stevkady@amazon.com>
>> Reviewed-by: Steven Noonan <snoonan@amazon.com>
>> Acked-by: Matt Wilson <msw@amazon.com>
>
> Not sure Acked-by by a non-maintainer makes much sense. Matt, did you
> mean Reviewed-by here instead?
>
If there are no other comments, should I send a [PATCH v3] with an
updated reviewers list?
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-12 19:25 ` [PATCH v2] " Valentin Priescu
2014-05-13 9:32 ` David Vrabel
@ 2014-05-20 20:28 ` Valentin Priescu
2014-05-22 9:11 ` Vasiliy Tolstov
2014-06-02 16:13 ` Valentin Priescu
1 sibling, 2 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-05-20 20:28 UTC (permalink / raw)
To: xen-devel; +Cc: Valentin Priescu, David Vrabel, Matt Wilson
From: Valentin Priescu <priescuv@amazon.com>
Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
requests to finish. If the VBD is attached to a hot-swappable disk, then
xenwatch can hang for a long period of time, stalling other watches.
INFO: task xenwatch:39 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
Call Trace:
[<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
[<ffffffff8128f761>] ? list_del+0x11/0x40
[<ffffffff8147a080>] ? wait_for_common+0x60/0x160
[<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
[<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
[<ffffffff8147a26a>] schedule+0x3a/0x60
[<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
[<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
[<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
[<ffffffff81346488>] device_release_driver+0x28/0x40
[<ffffffff813456e8>] bus_remove_device+0x78/0xe0
[<ffffffff81342c9f>] device_del+0x12f/0x1a0
[<ffffffff81342d2d>] device_unregister+0x1d/0x60
[<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
[<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
[<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
[<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
[<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
[<ffffffff8130b4cb>] frontend_changed+0xb/0x10
[<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
[<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
[<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
[<ffffffff810799d6>] kthread+0x96/0xa0
[<ffffffff81485934>] kernel_thread_helper+0x4/0x10
[<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
[<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
[<ffffffff81485930>] ? gs_change+0x13/0x13
With this patch, when there is still pending I/O, the actual disconnect
is done by the last reference holder (last pending I/O request). In this
case, xenwatch doesn't block indefinitely.
Signed-off-by: Valentin Priescu <priescuv@amazon.com>
Reviewed-by: Steven Kady <stevkady@amazon.com>
Reviewed-by: Steven Noonan <snoonan@amazon.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
v2: Reworked an 'if' statement in xen_blkif_disconnect(), as
suggested by David Vrabel <david.vrabel@citrix.com>
Handle the case when xen_blkif_disconnect() returns -EBUSY on
frontend_changed(), as noted by David Vrabel <david.vrabel@citrix.com>
v3: Update reviewers list.
drivers/block/xen-blkback/common.h | 4 ++--
drivers/block/xen-blkback/xenbus.c | 46 ++++++++++++++++++++++++++++----------
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index be05277..f65b807 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -314,7 +314,7 @@ struct xen_blkif {
unsigned long long st_rd_sect;
unsigned long long st_wr_sect;
- wait_queue_head_t waiting_to_free;
+ struct work_struct free_work;
/* Thread shutdown wait queue. */
wait_queue_head_t shutdown_wq;
};
@@ -361,7 +361,7 @@ struct pending_req {
#define xen_blkif_put(_b) \
do { \
if (atomic_dec_and_test(&(_b)->refcnt)) \
- wake_up(&(_b)->waiting_to_free);\
+ schedule_work(&(_b)->free_work);\
} while (0)
struct phys_req {
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 9a547e6..e911d28 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -35,12 +35,26 @@ static void connect(struct backend_info *);
static int connect_ring(struct backend_info *);
static void backend_changed(struct xenbus_watch *, const char **,
unsigned int);
+static void xen_blkif_free(struct xen_blkif *blkif);
+static void xen_vbd_free(struct xen_vbd *vbd);
struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
{
return be->dev;
}
+/*
+ * The last request could free the device from softirq context and
+ * xen_blkif_free() can sleep.
+ */
+static void xen_blkif_deferred_free(struct work_struct *work)
+{
+ struct xen_blkif *blkif;
+
+ blkif = container_of(work, struct xen_blkif, free_work);
+ xen_blkif_free(blkif);
+}
+
static int blkback_name(struct xen_blkif *blkif, char *buf)
{
char *devpath, *devname;
@@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
init_completion(&blkif->drain_complete);
atomic_set(&blkif->drain, 0);
blkif->st_print = jiffies;
- init_waitqueue_head(&blkif->waiting_to_free);
blkif->persistent_gnts.rb_node = NULL;
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
@@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
INIT_LIST_HEAD(&blkif->pending_free);
+ INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
for (i = 0; i < XEN_BLKIF_REQS; i++) {
req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
return 0;
}
-static void xen_blkif_disconnect(struct xen_blkif *blkif)
+static int xen_blkif_disconnect(struct xen_blkif *blkif)
{
if (blkif->xenblkd) {
kthread_stop(blkif->xenblkd);
@@ -239,9 +253,12 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
blkif->xenblkd = NULL;
}
- atomic_dec(&blkif->refcnt);
- wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
- atomic_inc(&blkif->refcnt);
+ /* The above kthread_stop() guarantees that at this point we
+ * don't have any discard_io or other_io requests. So, checking
+ * for inflight IO is enough.
+ */
+ if (atomic_read(&blkif->inflight) > 0)
+ return -EBUSY;
if (blkif->irq) {
unbind_from_irqhandler(blkif->irq, blkif);
@@ -252,6 +269,8 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
blkif->blk_rings.common.sring = NULL;
}
+
+ return 0;
}
static void xen_blkif_free(struct xen_blkif *blkif)
@@ -259,8 +278,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
struct pending_req *req, *n;
int i = 0, j;
- if (!atomic_dec_and_test(&blkif->refcnt))
- BUG();
+ xen_blkif_disconnect(blkif);
+ xen_vbd_free(&blkif->vbd);
/* Remove all persistent grants and the cache of ballooned pages. */
xen_blkbk_free_caches(blkif);
@@ -449,16 +468,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
be->backend_watch.node = NULL;
}
+ dev_set_drvdata(&dev->dev, NULL);
+
if (be->blkif) {
xen_blkif_disconnect(be->blkif);
- xen_vbd_free(&be->blkif->vbd);
- xen_blkif_free(be->blkif);
- be->blkif = NULL;
+ xen_blkif_put(be->blkif);
}
kfree(be->mode);
kfree(be);
- dev_set_drvdata(&dev->dev, NULL);
return 0;
}
@@ -700,7 +718,11 @@ static void frontend_changed(struct xenbus_device *dev,
* Enforce precondition before potential leak point.
* xen_blkif_disconnect() is idempotent.
*/
- xen_blkif_disconnect(be->blkif);
+ err = xen_blkif_disconnect(be->blkif);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "pending I/O");
+ break;
+ }
err = connect_ring(be);
if (err)
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-20 20:28 ` [PATCH v3] " Valentin Priescu
@ 2014-05-22 9:11 ` Vasiliy Tolstov
2014-05-22 10:16 ` Valentin Priescu
2014-06-02 16:13 ` Valentin Priescu
1 sibling, 1 reply; 27+ messages in thread
From: Vasiliy Tolstov @ 2014-05-22 9:11 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
I'm try to combine this patch and previous, but after 5-7 days of
using i have another not disconnectable devices.
My patch for 3.10.x https://gist.github.com/raw/3d7009a18dc920ba64fe
2014-05-21 0:28 GMT+04:00 Valentin Priescu <vali.priescu@gmail.com>:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
> INFO: task xenwatch:39 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
> Call Trace:
> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
> [<ffffffff8128f761>] ? list_del+0x11/0x40
> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
> [<ffffffff8147a26a>] schedule+0x3a/0x60
> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
> [<ffffffff81346488>] device_release_driver+0x28/0x40
> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
> [<ffffffff810799d6>] kthread+0x96/0xa0
> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
> [<ffffffff81485930>] ? gs_change+0x13/0x13
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
>
> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> Reviewed-by: Steven Kady <stevkady@amazon.com>
> Reviewed-by: Steven Noonan <snoonan@amazon.com>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
> ---
> v2: Reworked an 'if' statement in xen_blkif_disconnect(), as
> suggested by David Vrabel <david.vrabel@citrix.com>
> Handle the case when xen_blkif_disconnect() returns -EBUSY on
> frontend_changed(), as noted by David Vrabel <david.vrabel@citrix.com>
> v3: Update reviewers list.
>
> drivers/block/xen-blkback/common.h | 4 ++--
> drivers/block/xen-blkback/xenbus.c | 46 ++++++++++++++++++++++++++++----------
> 2 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
> index be05277..f65b807 100644
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -314,7 +314,7 @@ struct xen_blkif {
> unsigned long long st_rd_sect;
> unsigned long long st_wr_sect;
>
> - wait_queue_head_t waiting_to_free;
> + struct work_struct free_work;
> /* Thread shutdown wait queue. */
> wait_queue_head_t shutdown_wq;
> };
> @@ -361,7 +361,7 @@ struct pending_req {
> #define xen_blkif_put(_b) \
> do { \
> if (atomic_dec_and_test(&(_b)->refcnt)) \
> - wake_up(&(_b)->waiting_to_free);\
> + schedule_work(&(_b)->free_work);\
> } while (0)
>
> struct phys_req {
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 9a547e6..e911d28 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -35,12 +35,26 @@ static void connect(struct backend_info *);
> static int connect_ring(struct backend_info *);
> static void backend_changed(struct xenbus_watch *, const char **,
> unsigned int);
> +static void xen_blkif_free(struct xen_blkif *blkif);
> +static void xen_vbd_free(struct xen_vbd *vbd);
>
> struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
> {
> return be->dev;
> }
>
> +/*
> + * The last request could free the device from softirq context and
> + * xen_blkif_free() can sleep.
> + */
> +static void xen_blkif_deferred_free(struct work_struct *work)
> +{
> + struct xen_blkif *blkif;
> +
> + blkif = container_of(work, struct xen_blkif, free_work);
> + xen_blkif_free(blkif);
> +}
> +
> static int blkback_name(struct xen_blkif *blkif, char *buf)
> {
> char *devpath, *devname;
> @@ -121,7 +135,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> init_completion(&blkif->drain_complete);
> atomic_set(&blkif->drain, 0);
> blkif->st_print = jiffies;
> - init_waitqueue_head(&blkif->waiting_to_free);
> blkif->persistent_gnts.rb_node = NULL;
> spin_lock_init(&blkif->free_pages_lock);
> INIT_LIST_HEAD(&blkif->free_pages);
> @@ -132,6 +145,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>
> INIT_LIST_HEAD(&blkif->pending_free);
> + INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>
> for (i = 0; i < XEN_BLKIF_REQS; i++) {
> req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -231,7 +245,7 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned long shared_page,
> return 0;
> }
>
> -static void xen_blkif_disconnect(struct xen_blkif *blkif)
> +static int xen_blkif_disconnect(struct xen_blkif *blkif)
> {
> if (blkif->xenblkd) {
> kthread_stop(blkif->xenblkd);
> @@ -239,9 +253,12 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
> blkif->xenblkd = NULL;
> }
>
> - atomic_dec(&blkif->refcnt);
> - wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
> - atomic_inc(&blkif->refcnt);
> + /* The above kthread_stop() guarantees that at this point we
> + * don't have any discard_io or other_io requests. So, checking
> + * for inflight IO is enough.
> + */
> + if (atomic_read(&blkif->inflight) > 0)
> + return -EBUSY;
>
> if (blkif->irq) {
> unbind_from_irqhandler(blkif->irq, blkif);
> @@ -252,6 +269,8 @@ static void xen_blkif_disconnect(struct xen_blkif *blkif)
> xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
> blkif->blk_rings.common.sring = NULL;
> }
> +
> + return 0;
> }
>
> static void xen_blkif_free(struct xen_blkif *blkif)
> @@ -259,8 +278,8 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> struct pending_req *req, *n;
> int i = 0, j;
>
> - if (!atomic_dec_and_test(&blkif->refcnt))
> - BUG();
> + xen_blkif_disconnect(blkif);
> + xen_vbd_free(&blkif->vbd);
>
> /* Remove all persistent grants and the cache of ballooned pages. */
> xen_blkbk_free_caches(blkif);
> @@ -449,16 +468,15 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
> be->backend_watch.node = NULL;
> }
>
> + dev_set_drvdata(&dev->dev, NULL);
> +
> if (be->blkif) {
> xen_blkif_disconnect(be->blkif);
> - xen_vbd_free(&be->blkif->vbd);
> - xen_blkif_free(be->blkif);
> - be->blkif = NULL;
> + xen_blkif_put(be->blkif);
> }
>
> kfree(be->mode);
> kfree(be);
> - dev_set_drvdata(&dev->dev, NULL);
> return 0;
> }
>
> @@ -700,7 +718,11 @@ static void frontend_changed(struct xenbus_device *dev,
> * Enforce precondition before potential leak point.
> * xen_blkif_disconnect() is idempotent.
> */
> - xen_blkif_disconnect(be->blkif);
> + err = xen_blkif_disconnect(be->blkif);
> + if (err) {
> + xenbus_dev_fatal(dev, err, "pending I/O");
> + break;
> + }
>
> err = connect_ring(be);
> if (err)
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
--
Vasiliy Tolstov,
e-mail: v.tolstov@selfip.ru
jabber: vase@selfip.ru
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-22 9:11 ` Vasiliy Tolstov
@ 2014-05-22 10:16 ` Valentin Priescu
2014-06-03 8:07 ` Vasiliy Tolstov
0 siblings, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-05-22 10:16 UTC (permalink / raw)
To: Vasiliy Tolstov; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
On Thu, May 22, 2014 at 11:11 AM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote:
> I'm try to combine this patch and previous, but after 5-7 days of
> using i have another not disconnectable devices.
> My patch for 3.10.x https://gist.github.com/raw/3d7009a18dc920ba64fe
>
Hi Vasiliy,
Can you describe the test you're trying to run?
What exactly do you mean by "i have another not disconnectable devices."?
--
Valentin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-20 20:28 ` [PATCH v3] " Valentin Priescu
2014-05-22 9:11 ` Vasiliy Tolstov
@ 2014-06-02 16:13 ` Valentin Priescu
2014-06-02 16:56 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 27+ messages in thread
From: Valentin Priescu @ 2014-06-02 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Valentin Priescu, David Vrabel, Matt Wilson
Hi Konrad,
Do you have any comments regarding this patch?
--
Valentin
On Tue, May 20, 2014 at 10:28 PM, Valentin Priescu
<vali.priescu@gmail.com> wrote:
> From: Valentin Priescu <priescuv@amazon.com>
>
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
> INFO: task xenwatch:39 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
> ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
> ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
> Call Trace:
> [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
> [<ffffffff8128f761>] ? list_del+0x11/0x40
> [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
> [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
> [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
> [<ffffffff8147a26a>] schedule+0x3a/0x60
> [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
> [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
> [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
> [<ffffffff81346488>] device_release_driver+0x28/0x40
> [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
> [<ffffffff81342c9f>] device_del+0x12f/0x1a0
> [<ffffffff81342d2d>] device_unregister+0x1d/0x60
> [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
> [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
> [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
> [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
> [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
> [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
> [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
> [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
> [<ffffffff810799d6>] kthread+0x96/0xa0
> [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
> [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
> [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
> [<ffffffff81485930>] ? gs_change+0x13/0x13
>
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.
>
> Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> Reviewed-by: Steven Kady <stevkady@amazon.com>
> Reviewed-by: Steven Noonan <snoonan@amazon.com>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
> ---
> v2: Reworked an 'if' statement in xen_blkif_disconnect(), as
> suggested by David Vrabel <david.vrabel@citrix.com>
> Handle the case when xen_blkif_disconnect() returns -EBUSY on
> frontend_changed(), as noted by David Vrabel <david.vrabel@citrix.com>
> v3: Update reviewers list.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-13 9:32 ` David Vrabel
2014-05-13 17:00 ` Vasiliy Tolstov
2014-05-16 8:37 ` Valentin Priescu
@ 2014-06-02 16:20 ` Matt Wilson
2 siblings, 0 replies; 27+ messages in thread
From: Matt Wilson @ 2014-06-02 16:20 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Matt Wilson, Valentin Priescu, Valentin Priescu
On Tue, May 13, 2014 at 10:32:36AM +0100, David Vrabel wrote:
> On 12/05/14 20:25, Valentin Priescu wrote:
> > From: Valentin Priescu <priescuv@amazon.com>
> >
> > Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> > requests to finish. If the VBD is attached to a hot-swappable disk, then
> > xenwatch can hang for a long period of time, stalling other watches.
> >
> [...]
> >
> > With this patch, when there is still pending I/O, the actual disconnect
> > is done by the last reference holder (last pending I/O request). In this
> > case, xenwatch doesn't block indefinitely.
>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
>
> > Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> > Reviewed-by: Steven Kady <stevkady@amazon.com>
> > Reviewed-by: Steven Noonan <snoonan@amazon.com>
> > Acked-by: Matt Wilson <msw@amazon.com>
>
> Not sure Acked-by by a non-maintainer makes much sense. Matt, did you
> mean Reviewed-by here instead?
Apologies for the late reply.
I had not performed a thorough review of this form of the patch, but I
wanted to show that I think that the patch is needed and should be
applied. Acked-by: isn't only used by maintainers.
--msw
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-06-02 16:13 ` Valentin Priescu
@ 2014-06-02 16:56 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-06-02 16:56 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
On Mon, Jun 02, 2014 at 06:13:22PM +0200, Valentin Priescu wrote:
> Hi Konrad,
>
> Do you have any comments regarding this patch?
No. Patch had been already pulled by Jens for 3.16.
>
> --
> Valentin
>
> On Tue, May 20, 2014 at 10:28 PM, Valentin Priescu
> <vali.priescu@gmail.com> wrote:
> > From: Valentin Priescu <priescuv@amazon.com>
> >
> > Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> > requests to finish. If the VBD is attached to a hot-swappable disk, then
> > xenwatch can hang for a long period of time, stalling other watches.
> >
> > INFO: task xenwatch:39 blocked for more than 120 seconds.
> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > ffff880057f01bd0 0000000000000246 ffff880057f01ac0 ffffffff810b0782
> > ffff880057f01ad0 00000000000131c0 0000000000000004 ffff880057edb040
> > ffff8800344c6080 0000000000000000 ffff880058c00ba0 ffff880057edb040
> > Call Trace:
> > [<ffffffff810b0782>] ? irq_to_desc+0x12/0x20
> > [<ffffffff8128f761>] ? list_del+0x11/0x40
> > [<ffffffff8147a080>] ? wait_for_common+0x60/0x160
> > [<ffffffff8147bcef>] ? _raw_spin_lock_irqsave+0x2f/0x50
> > [<ffffffff8147bd49>] ? _raw_spin_unlock_irqrestore+0x19/0x20
> > [<ffffffff8147a26a>] schedule+0x3a/0x60
> > [<ffffffffa018fe6a>] xen_blkif_disconnect+0x8a/0x100 [xen_blkback]
> > [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> > [<ffffffffa018ffce>] xen_blkbk_remove+0xae/0x1e0 [xen_blkback]
> > [<ffffffff8130b254>] xenbus_dev_remove+0x44/0x90
> > [<ffffffff81345cb7>] __device_release_driver+0x77/0xd0
> > [<ffffffff81346488>] device_release_driver+0x28/0x40
> > [<ffffffff813456e8>] bus_remove_device+0x78/0xe0
> > [<ffffffff81342c9f>] device_del+0x12f/0x1a0
> > [<ffffffff81342d2d>] device_unregister+0x1d/0x60
> > [<ffffffffa0190826>] frontend_changed+0xa6/0x4d0 [xen_blkback]
> > [<ffffffffa019c252>] ? frontend_changed+0x192/0x650 [xen_netback]
> > [<ffffffff8130ae50>] ? cmp_dev+0x60/0x60
> > [<ffffffff81344fe4>] ? bus_for_each_dev+0x94/0xa0
> > [<ffffffff8130b06e>] xenbus_otherend_changed+0xbe/0x120
> > [<ffffffff8130b4cb>] frontend_changed+0xb/0x10
> > [<ffffffff81309c82>] xenwatch_thread+0xf2/0x130
> > [<ffffffff81079f70>] ? wake_up_bit+0x40/0x40
> > [<ffffffff81309b90>] ? xenbus_directory+0x80/0x80
> > [<ffffffff810799d6>] kthread+0x96/0xa0
> > [<ffffffff81485934>] kernel_thread_helper+0x4/0x10
> > [<ffffffff814839f3>] ? int_ret_from_sys_call+0x7/0x1b
> > [<ffffffff8147c17c>] ? retint_restore_args+0x5/0x6
> > [<ffffffff81485930>] ? gs_change+0x13/0x13
> >
> > With this patch, when there is still pending I/O, the actual disconnect
> > is done by the last reference holder (last pending I/O request). In this
> > case, xenwatch doesn't block indefinitely.
> >
> > Signed-off-by: Valentin Priescu <priescuv@amazon.com>
> > Reviewed-by: Steven Kady <stevkady@amazon.com>
> > Reviewed-by: Steven Noonan <snoonan@amazon.com>
> > Reviewed-by: David Vrabel <david.vrabel@citrix.com>
> > ---
> > v2: Reworked an 'if' statement in xen_blkif_disconnect(), as
> > suggested by David Vrabel <david.vrabel@citrix.com>
> > Handle the case when xen_blkif_disconnect() returns -EBUSY on
> > frontend_changed(), as noted by David Vrabel <david.vrabel@citrix.com>
> > v3: Update reviewers list.
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-05-22 10:16 ` Valentin Priescu
@ 2014-06-03 8:07 ` Vasiliy Tolstov
2014-06-03 9:59 ` Valentin Priescu
0 siblings, 1 reply; 27+ messages in thread
From: Vasiliy Tolstov @ 2014-06-03 8:07 UTC (permalink / raw)
To: Valentin Priescu; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
[-- Attachment #1.1: Type: text/plain, Size: 714 bytes --]
2014-05-22 14:16 GMT+04:00 Valentin Priescu <vali.priescu@gmail.com>:
> Hi Vasiliy,
>
> Can you describe the test you're trying to run?
> What exactly do you mean by "i have another not disconnectable devices."?
>
I have latest 3.10.x kernel with this patch (link is
https://gist.github.com/raw/3d7009a18dc920ba64fe)
I'm create md device (raid1) /dev/md121 and start vm.
After it shutdown my block script can't stop array because device or
resource busy.
Domains dies :
xl list
(null) 9 0 8 --p--d
129291.0
(null) 2 0 4 --ps-d
1196.2
--
Vasiliy Tolstov,
e-mail: v.tolstov@selfip.ru
jabber: vase@selfip.ru
[-- Attachment #1.2: Type: text/html, Size: 1791 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen-blkback: defer freeing blkif to avoid blocking xenwatch
2014-06-03 8:07 ` Vasiliy Tolstov
@ 2014-06-03 9:59 ` Valentin Priescu
0 siblings, 0 replies; 27+ messages in thread
From: Valentin Priescu @ 2014-06-03 9:59 UTC (permalink / raw)
To: Vasiliy Tolstov; +Cc: xen-devel, David Vrabel, Matt Wilson, Valentin Priescu
Hi Vasiliy,
Can you tell me more about the state of the md array? Are there any
requests pending on the device?
Did you suspend the array before you shutdown the domain? What's the
open count of the md device?
>From your previous description, looks like the device still has
pending IO and the domain couldn't be entirely destroyed. Or something
messed up the refcnt and the blkif/vbd was never released.
--
Valentin
On Tue, Jun 3, 2014 at 10:07 AM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote:
>
>
> I have latest 3.10.x kernel with this patch (link is
> https://gist.github.com/raw/3d7009a18dc920ba64fe)
> I'm create md device (raid1) /dev/md121 and start vm.
> After it shutdown my block script can't stop array because device or
> resource busy.
> Domains dies :
> xl list
> (null) 9 0 8 --p--d
> 129291.0
> (null) 2 0 4 --ps-d
> 1196.2
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2014-06-03 9:59 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-09 23:34 [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch Valentin Priescu
2014-05-09 23:56 ` Matt Wilson
2014-05-12 6:42 ` Vasiliy Tolstov
2014-05-12 8:59 ` Valentin Priescu
2014-05-12 9:12 ` Alexey
2014-05-12 10:50 ` Valentin Priescu
2014-05-12 9:39 ` David Vrabel
2014-05-12 10:41 ` Valentin Priescu
2014-05-12 10:43 ` David Vrabel
2014-05-12 10:47 ` Valentin Priescu
2014-05-12 16:40 ` Boris Ostrovsky
2014-05-12 17:04 ` Valentin Priescu
2014-05-12 17:50 ` Boris Ostrovsky
2014-05-12 19:25 ` [PATCH v2] " Valentin Priescu
2014-05-13 9:32 ` David Vrabel
2014-05-13 17:00 ` Vasiliy Tolstov
2014-05-13 17:32 ` Valentin Priescu
2014-05-13 17:39 ` Vasiliy Tolstov
2014-05-16 8:37 ` Valentin Priescu
2014-06-02 16:20 ` Matt Wilson
2014-05-20 20:28 ` [PATCH v3] " Valentin Priescu
2014-05-22 9:11 ` Vasiliy Tolstov
2014-05-22 10:16 ` Valentin Priescu
2014-06-03 8:07 ` Vasiliy Tolstov
2014-06-03 9:59 ` Valentin Priescu
2014-06-02 16:13 ` Valentin Priescu
2014-06-02 16:56 ` Konrad Rzeszutek Wilk
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).