All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected
@ 2014-09-08 13:21 Vitaly Kuznetsov
  2014-09-12 19:23 ` Konrad Rzeszutek Wilk
  2014-09-12 19:23 ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2014-09-08 13:21 UTC (permalink / raw)
  To: xen-devel
  Cc: Konrad Rzeszutek Wilk, David Vrabel, Roger Pau Monne,
	Andrew Jones, linux-kernel

blkback does not unmap persistent grants when frontend goes to Closed
state (e.g. when blkfront module is being removed). This leads to the
following in guest's dmesg:

[  343.243825] xen:grant_table: WARNING: g.e. 0x445 still in use!
[  343.243825] xen:grant_table: WARNING: g.e. 0x42a still in use!
...

When load module -> use device -> unload module sequence is performed multiple times
it is possible to hit BUG() condition in blkfront module:

[  343.243825] kernel BUG at drivers/block/xen-blkfront.c:954!
[  343.243825] invalid opcode: 0000 [#1] SMP
[  343.243825] Modules linked in: xen_blkfront(-) ata_generic pata_acpi [last unloaded: xen_blkfront]
...
[  343.243825] Call Trace:
[  343.243825]  [<ffffffff814111ef>] ? unregister_xenbus_watch+0x16f/0x1e0
[  343.243825]  [<ffffffffa0016fbf>] blkfront_remove+0x3f/0x140 [xen_blkfront]
...
[  343.243825] RIP  [<ffffffffa0016aae>] blkif_free+0x34e/0x360 [xen_blkfront]
[  343.243825]  RSP <ffff88001eb8fdc0>

We don't need to keep these grants if we're disconnecting as frontend might already
forgot about them. Solve the issue by moving xen_blkbk_free_caches() call from
xen_blkif_free() to xen_blkif_disconnect().

Now we can see the following:
[  928.590893] xen:grant_table: WARNING: g.e. 0x587 still in use!
[  928.591861] xen:grant_table: WARNING: g.e. 0x372 still in use!
...
[  929.592146] xen:grant_table: freeing g.e. 0x587
[  929.597174] xen:grant_table: freeing g.e. 0x372
...

Backend does not keep persistent grants any more, reconnect works fine.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/block/xen-blkback/xenbus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3a8b810..54f4089 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -270,6 +270,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 		blkif->blk_rings.common.sring = NULL;
 	}
 
+	/* Remove all persistent grants and the cache of ballooned pages. */
+	xen_blkbk_free_caches(blkif);
+
 	return 0;
 }
 
@@ -281,9 +284,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
 	xen_blkif_disconnect(blkif);
 	xen_vbd_free(&blkif->vbd);
 
-	/* Remove all persistent grants and the cache of ballooned pages. */
-	xen_blkbk_free_caches(blkif);
-
 	/* Make sure everything is drained before shutting down */
 	BUG_ON(blkif->persistent_gnt_c != 0);
 	BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
-- 
1.9.3


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

* Re: [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected
  2014-09-08 13:21 [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected Vitaly Kuznetsov
  2014-09-12 19:23 ` Konrad Rzeszutek Wilk
@ 2014-09-12 19:23 ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-09-12 19:23 UTC (permalink / raw)
  To: Vitaly Kuznetsov, david.vrabel
  Cc: xen-devel, David Vrabel, Roger Pau Monne, Andrew Jones, linux-kernel

On Mon, Sep 08, 2014 at 03:21:33PM +0200, Vitaly Kuznetsov wrote:
> blkback does not unmap persistent grants when frontend goes to Closed
> state (e.g. when blkfront module is being removed). This leads to the
> following in guest's dmesg:
> 
> [  343.243825] xen:grant_table: WARNING: g.e. 0x445 still in use!
> [  343.243825] xen:grant_table: WARNING: g.e. 0x42a still in use!
> ...
> 
> When load module -> use device -> unload module sequence is performed multiple times
> it is possible to hit BUG() condition in blkfront module:
> 
> [  343.243825] kernel BUG at drivers/block/xen-blkfront.c:954!
> [  343.243825] invalid opcode: 0000 [#1] SMP
> [  343.243825] Modules linked in: xen_blkfront(-) ata_generic pata_acpi [last unloaded: xen_blkfront]
> ...
> [  343.243825] Call Trace:
> [  343.243825]  [<ffffffff814111ef>] ? unregister_xenbus_watch+0x16f/0x1e0
> [  343.243825]  [<ffffffffa0016fbf>] blkfront_remove+0x3f/0x140 [xen_blkfront]
> ...
> [  343.243825] RIP  [<ffffffffa0016aae>] blkif_free+0x34e/0x360 [xen_blkfront]
> [  343.243825]  RSP <ffff88001eb8fdc0>
> 
> We don't need to keep these grants if we're disconnecting as frontend might already
> forgot about them. Solve the issue by moving xen_blkbk_free_caches() call from
> xen_blkif_free() to xen_blkif_disconnect().
> 
> Now we can see the following:
> [  928.590893] xen:grant_table: WARNING: g.e. 0x587 still in use!
> [  928.591861] xen:grant_table: WARNING: g.e. 0x372 still in use!
> ...
> [  929.592146] xen:grant_table: freeing g.e. 0x587
> [  929.597174] xen:grant_table: freeing g.e. 0x372
> ...
> 
> Backend does not keep persistent grants any more, reconnect works fine.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

David,

I usually combine the 'block' related patches in one branch and send them to Jens
for the merge window.

Want to do it the same way or do you want to alter the workflow?

> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  drivers/block/xen-blkback/xenbus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 3a8b810..54f4089 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -270,6 +270,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
>  		blkif->blk_rings.common.sring = NULL;
>  	}
>  
> +	/* Remove all persistent grants and the cache of ballooned pages. */
> +	xen_blkbk_free_caches(blkif);
> +
>  	return 0;
>  }
>  
> @@ -281,9 +284,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>  	xen_blkif_disconnect(blkif);
>  	xen_vbd_free(&blkif->vbd);
>  
> -	/* Remove all persistent grants and the cache of ballooned pages. */
> -	xen_blkbk_free_caches(blkif);
> -
>  	/* Make sure everything is drained before shutting down */
>  	BUG_ON(blkif->persistent_gnt_c != 0);
>  	BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
> -- 
> 1.9.3
> 

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

* Re: [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected
  2014-09-08 13:21 [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected Vitaly Kuznetsov
@ 2014-09-12 19:23 ` Konrad Rzeszutek Wilk
  2014-09-12 19:23 ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-09-12 19:23 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: xen-devel, Andrew Jones, David Vrabel, linux-kernel, Roger Pau Monne

On Mon, Sep 08, 2014 at 03:21:33PM +0200, Vitaly Kuznetsov wrote:
> blkback does not unmap persistent grants when frontend goes to Closed
> state (e.g. when blkfront module is being removed). This leads to the
> following in guest's dmesg:
> 
> [  343.243825] xen:grant_table: WARNING: g.e. 0x445 still in use!
> [  343.243825] xen:grant_table: WARNING: g.e. 0x42a still in use!
> ...
> 
> When load module -> use device -> unload module sequence is performed multiple times
> it is possible to hit BUG() condition in blkfront module:
> 
> [  343.243825] kernel BUG at drivers/block/xen-blkfront.c:954!
> [  343.243825] invalid opcode: 0000 [#1] SMP
> [  343.243825] Modules linked in: xen_blkfront(-) ata_generic pata_acpi [last unloaded: xen_blkfront]
> ...
> [  343.243825] Call Trace:
> [  343.243825]  [<ffffffff814111ef>] ? unregister_xenbus_watch+0x16f/0x1e0
> [  343.243825]  [<ffffffffa0016fbf>] blkfront_remove+0x3f/0x140 [xen_blkfront]
> ...
> [  343.243825] RIP  [<ffffffffa0016aae>] blkif_free+0x34e/0x360 [xen_blkfront]
> [  343.243825]  RSP <ffff88001eb8fdc0>
> 
> We don't need to keep these grants if we're disconnecting as frontend might already
> forgot about them. Solve the issue by moving xen_blkbk_free_caches() call from
> xen_blkif_free() to xen_blkif_disconnect().
> 
> Now we can see the following:
> [  928.590893] xen:grant_table: WARNING: g.e. 0x587 still in use!
> [  928.591861] xen:grant_table: WARNING: g.e. 0x372 still in use!
> ...
> [  929.592146] xen:grant_table: freeing g.e. 0x587
> [  929.597174] xen:grant_table: freeing g.e. 0x372
> ...
> 
> Backend does not keep persistent grants any more, reconnect works fine.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

David,

I usually combine the 'block' related patches in one branch and send them to Jens
for the merge window.

Want to do it the same way or do you want to alter the workflow?

> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  drivers/block/xen-blkback/xenbus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 3a8b810..54f4089 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -270,6 +270,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
>  		blkif->blk_rings.common.sring = NULL;
>  	}
>  
> +	/* Remove all persistent grants and the cache of ballooned pages. */
> +	xen_blkbk_free_caches(blkif);
> +
>  	return 0;
>  }
>  
> @@ -281,9 +284,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>  	xen_blkif_disconnect(blkif);
>  	xen_vbd_free(&blkif->vbd);
>  
> -	/* Remove all persistent grants and the cache of ballooned pages. */
> -	xen_blkbk_free_caches(blkif);
> -
>  	/* Make sure everything is drained before shutting down */
>  	BUG_ON(blkif->persistent_gnt_c != 0);
>  	BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
> -- 
> 1.9.3
> 

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

* [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected
@ 2014-09-08 13:21 Vitaly Kuznetsov
  0 siblings, 0 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2014-09-08 13:21 UTC (permalink / raw)
  To: xen-devel; +Cc: linux-kernel, Andrew Jones, Roger Pau Monne, David Vrabel

blkback does not unmap persistent grants when frontend goes to Closed
state (e.g. when blkfront module is being removed). This leads to the
following in guest's dmesg:

[  343.243825] xen:grant_table: WARNING: g.e. 0x445 still in use!
[  343.243825] xen:grant_table: WARNING: g.e. 0x42a still in use!
...

When load module -> use device -> unload module sequence is performed multiple times
it is possible to hit BUG() condition in blkfront module:

[  343.243825] kernel BUG at drivers/block/xen-blkfront.c:954!
[  343.243825] invalid opcode: 0000 [#1] SMP
[  343.243825] Modules linked in: xen_blkfront(-) ata_generic pata_acpi [last unloaded: xen_blkfront]
...
[  343.243825] Call Trace:
[  343.243825]  [<ffffffff814111ef>] ? unregister_xenbus_watch+0x16f/0x1e0
[  343.243825]  [<ffffffffa0016fbf>] blkfront_remove+0x3f/0x140 [xen_blkfront]
...
[  343.243825] RIP  [<ffffffffa0016aae>] blkif_free+0x34e/0x360 [xen_blkfront]
[  343.243825]  RSP <ffff88001eb8fdc0>

We don't need to keep these grants if we're disconnecting as frontend might already
forgot about them. Solve the issue by moving xen_blkbk_free_caches() call from
xen_blkif_free() to xen_blkif_disconnect().

Now we can see the following:
[  928.590893] xen:grant_table: WARNING: g.e. 0x587 still in use!
[  928.591861] xen:grant_table: WARNING: g.e. 0x372 still in use!
...
[  929.592146] xen:grant_table: freeing g.e. 0x587
[  929.597174] xen:grant_table: freeing g.e. 0x372
...

Backend does not keep persistent grants any more, reconnect works fine.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/block/xen-blkback/xenbus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3a8b810..54f4089 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -270,6 +270,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 		blkif->blk_rings.common.sring = NULL;
 	}
 
+	/* Remove all persistent grants and the cache of ballooned pages. */
+	xen_blkbk_free_caches(blkif);
+
 	return 0;
 }
 
@@ -281,9 +284,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
 	xen_blkif_disconnect(blkif);
 	xen_vbd_free(&blkif->vbd);
 
-	/* Remove all persistent grants and the cache of ballooned pages. */
-	xen_blkbk_free_caches(blkif);
-
 	/* Make sure everything is drained before shutting down */
 	BUG_ON(blkif->persistent_gnt_c != 0);
 	BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
-- 
1.9.3

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

end of thread, other threads:[~2014-09-12 19:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-08 13:21 [PATCH] xen/blkback: unmap all persistent grants when frontend gets disconnected Vitaly Kuznetsov
2014-09-12 19:23 ` Konrad Rzeszutek Wilk
2014-09-12 19:23 ` Konrad Rzeszutek Wilk
2014-09-08 13:21 Vitaly Kuznetsov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.