* Re: [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
@ 2015-08-10 18:38 ` Boris Ostrovsky
2015-08-10 18:38 ` Boris Ostrovsky
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Boris Ostrovsky @ 2015-08-10 18:38 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: David Vrabel, Wei Liu, linux-kernel
On 08/10/2015 02:10 PM, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
> Appeared in Linux 4.1. HVM backend, which is always the case on ARM, will
> leak every mapped ring (i.e ~12KB per domain with 1 disk and 1 vif).
> ---
> drivers/xen/xenbus/xenbus_client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 9ad3272..e303535 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -814,8 +814,10 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr)
>
> rv = xenbus_unmap_ring(dev, node->handles, node->nr_handles,
> addrs);
> - if (!rv)
> + if (!rv) {
> vunmap(vaddr);
> + free_xenballooned_pages(node->nr_handles, node->hvm.pages);
> + }
> else
> WARN(1, "Leaking %p, size %u page(s)\n", vaddr,
> node->nr_handles);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
2015-08-10 18:38 ` Boris Ostrovsky
@ 2015-08-10 18:38 ` Boris Ostrovsky
2015-08-10 18:46 ` Wei Liu
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Boris Ostrovsky @ 2015-08-10 18:38 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: linux-kernel, Konrad Rzeszutek Wilk, David Vrabel, Wei Liu
On 08/10/2015 02:10 PM, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
> Appeared in Linux 4.1. HVM backend, which is always the case on ARM, will
> leak every mapped ring (i.e ~12KB per domain with 1 disk and 1 vif).
> ---
> drivers/xen/xenbus/xenbus_client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 9ad3272..e303535 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -814,8 +814,10 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr)
>
> rv = xenbus_unmap_ring(dev, node->handles, node->nr_handles,
> addrs);
> - if (!rv)
> + if (!rv) {
> vunmap(vaddr);
> + free_xenballooned_pages(node->nr_handles, node->hvm.pages);
> + }
> else
> WARN(1, "Leaking %p, size %u page(s)\n", vaddr,
> node->nr_handles);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
2015-08-10 18:38 ` Boris Ostrovsky
2015-08-10 18:38 ` Boris Ostrovsky
@ 2015-08-10 18:46 ` Wei Liu
2015-08-10 18:46 ` Wei Liu
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2015-08-10 18:46 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, linux-kernel, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Wei Liu
On Mon, Aug 10, 2015 at 07:10:38PM +0100, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> Appeared in Linux 4.1. HVM backend, which is always the case on ARM, will
> leak every mapped ring (i.e ~12KB per domain with 1 disk and 1 vif).
> ---
> drivers/xen/xenbus/xenbus_client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 9ad3272..e303535 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -814,8 +814,10 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr)
>
> rv = xenbus_unmap_ring(dev, node->handles, node->nr_handles,
> addrs);
> - if (!rv)
> + if (!rv) {
> vunmap(vaddr);
> + free_xenballooned_pages(node->nr_handles, node->hvm.pages);
> + }
> else
> WARN(1, "Leaking %p, size %u page(s)\n", vaddr,
> node->nr_handles);
> --
> 2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
` (2 preceding siblings ...)
2015-08-10 18:46 ` Wei Liu
@ 2015-08-10 18:46 ` Wei Liu
2015-08-11 10:05 ` [Xen-devel] " David Vrabel
2015-08-11 10:05 ` David Vrabel
5 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2015-08-10 18:46 UTC (permalink / raw)
To: Julien Grall
Cc: Wei Liu, linux-kernel, David Vrabel, xen-devel, Boris Ostrovsky
On Mon, Aug 10, 2015 at 07:10:38PM +0100, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> Appeared in Linux 4.1. HVM backend, which is always the case on ARM, will
> leak every mapped ring (i.e ~12KB per domain with 1 disk and 1 vif).
> ---
> drivers/xen/xenbus/xenbus_client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 9ad3272..e303535 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -814,8 +814,10 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr)
>
> rv = xenbus_unmap_ring(dev, node->handles, node->nr_handles,
> addrs);
> - if (!rv)
> + if (!rv) {
> vunmap(vaddr);
> + free_xenballooned_pages(node->nr_handles, node->hvm.pages);
> + }
> else
> WARN(1, "Leaking %p, size %u page(s)\n", vaddr,
> node->nr_handles);
> --
> 2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Xen-devel] [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
` (3 preceding siblings ...)
2015-08-10 18:46 ` Wei Liu
@ 2015-08-11 10:05 ` David Vrabel
2015-08-11 10:05 ` David Vrabel
5 siblings, 0 replies; 7+ messages in thread
From: David Vrabel @ 2015-08-11 10:05 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: Wei Liu, linux-kernel, David Vrabel, Boris Ostrovsky
On 10/08/15 19:10, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
Applied to for-linus-4.2 and tagged for stable, thanks.
David
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend
2015-08-10 18:10 [PATCH] xen/xenbus: Don't leak memory when unmapping the ring on HVM backend Julien Grall
` (4 preceding siblings ...)
2015-08-11 10:05 ` [Xen-devel] " David Vrabel
@ 2015-08-11 10:05 ` David Vrabel
5 siblings, 0 replies; 7+ messages in thread
From: David Vrabel @ 2015-08-11 10:05 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: Boris Ostrovsky, Wei Liu, linux-kernel, David Vrabel
On 10/08/15 19:10, Julien Grall wrote:
> The commit ccc9d90a9a8b5c4ad7e9708ec41f75ff9e98d61d "xenbus_client:
> Extend interface to support multi-page ring" removes the call to
> free_xenballooned_pages in xenbus_unmap_ring_vfree_hvm.
>
> This will result to not give back the pages to Linux and loose them
> forever. It only happens when the backends are running in HVM domains.
Applied to for-linus-4.2 and tagged for stable, thanks.
David
^ permalink raw reply [flat|nested] 7+ messages in thread