[PATCH v2] xl: free bitmaps on exit

[PATCH v2] xl: free bitmaps on exit

[Olaf Hering]

Every invocation of xl via valgrind will show three leaks.
Since libxl_bitmap_alloc uses NOGC, the caller has to free the memory
after use. And since xl_ctx_free might be called before
parse_global_config, also move the libxl_bitmap_init calls into
xl_ctx_alloc.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xl/xl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/xl/xl.c b/tools/xl/xl.c
index 7d2142f16f..f5a17cf0d1 100644
--- a/tools/xl/xl.c
+++ b/tools/xl/xl.c
@@ -209,11 +209,8 @@ static void parse_global_config(const char *configfile,
     if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
         max_maptrack_frames = l;
 
-    libxl_bitmap_init(&global_vm_affinity_mask);
     libxl_cpu_bitmap_alloc(ctx, &global_vm_affinity_mask, 0);
-    libxl_bitmap_init(&global_hvm_affinity_mask);
     libxl_cpu_bitmap_alloc(ctx, &global_hvm_affinity_mask, 0);
-    libxl_bitmap_init(&global_pv_affinity_mask);
     libxl_cpu_bitmap_alloc(ctx, &global_pv_affinity_mask, 0);
 
     if (!xlu_cfg_get_string (config, "vm.cpumask", &buf, 0))
@@ -323,11 +320,17 @@ void xl_ctx_alloc(void) {
         exit(1);
     }
 
+    libxl_bitmap_init(&global_vm_affinity_mask);
+    libxl_bitmap_init(&global_hvm_affinity_mask);
+    libxl_bitmap_init(&global_pv_affinity_mask);
     libxl_childproc_setmode(ctx, &childproc_hooks, 0);
 }
 
 static void xl_ctx_free(void)
 {
+    libxl_bitmap_dispose(&global_pv_affinity_mask);
+    libxl_bitmap_dispose(&global_hvm_affinity_mask);
+    libxl_bitmap_dispose(&global_vm_affinity_mask);
     if (ctx) {
         libxl_ctx_free(ctx);
         ctx = NULL;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xl: free bitmaps on exit

[Wei Liu]

On Tue, Nov 27, 2018 at 11:06:08AM +0100, Olaf Hering wrote:
> Every invocation of xl via valgrind will show three leaks.
> Since libxl_bitmap_alloc uses NOGC, the caller has to free the memory
> after use. And since xl_ctx_free might be called before
> parse_global_config, also move the libxl_bitmap_init calls into
> xl_ctx_alloc.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks for fixing this.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xl: free bitmaps on exit

[Wei Liu]

On Tue, Nov 27, 2018 at 11:06:08AM +0100, Olaf Hering wrote:
> Every invocation of xl via valgrind will show three leaks.
> Since libxl_bitmap_alloc uses NOGC, the caller has to free the memory
> after use. And since xl_ctx_free might be called before
> parse_global_config, also move the libxl_bitmap_init calls into
> xl_ctx_alloc.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/xl/xl.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/xl/xl.c b/tools/xl/xl.c
> index 7d2142f16f..f5a17cf0d1 100644
> --- a/tools/xl/xl.c
> +++ b/tools/xl/xl.c
> @@ -209,11 +209,8 @@ static void parse_global_config(const char *configfile,
>      if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
>          max_maptrack_frames = l;
>  
> -    libxl_bitmap_init(&global_vm_affinity_mask);
>      libxl_cpu_bitmap_alloc(ctx, &global_vm_affinity_mask, 0);
> -    libxl_bitmap_init(&global_hvm_affinity_mask);
>      libxl_cpu_bitmap_alloc(ctx, &global_hvm_affinity_mask, 0);
> -    libxl_bitmap_init(&global_pv_affinity_mask);
>      libxl_cpu_bitmap_alloc(ctx, &global_pv_affinity_mask, 0);
>  
>      if (!xlu_cfg_get_string (config, "vm.cpumask", &buf, 0))
> @@ -323,11 +320,17 @@ void xl_ctx_alloc(void) {
>          exit(1);
>      }
>  
> +    libxl_bitmap_init(&global_vm_affinity_mask);
> +    libxl_bitmap_init(&global_hvm_affinity_mask);
> +    libxl_bitmap_init(&global_pv_affinity_mask);

Looking more closely into this, these lines should be moved even
earlier before the call to libxl_ctx_alloc -- there is an exit there.

Sorry for not having noticed this earlier before giving my ack. Please
submit v3.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xl: free bitmaps on exit

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 387 bytes --]

Am Tue, 27 Nov 2018 16:55:38 +0000
schrieb Wei Liu <wei.liu2@citrix.com>:

> Looking more closely into this, these lines should be moved even
> earlier before the call to libxl_ctx_alloc -- there is an exit there.

Is it required to install the atexit handler before alloc()?
To me it looks like atexit(xl_ctx_free) should be done right after, or inside, xl_ctx_alloc().

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xl: free bitmaps on exit

[Wei Liu]

On Tue, Nov 27, 2018 at 06:09:33PM +0100, Olaf Hering wrote:
> Am Tue, 27 Nov 2018 16:55:38 +0000
> schrieb Wei Liu <wei.liu2@citrix.com>:
> 
> > Looking more closely into this, these lines should be moved even
> > earlier before the call to libxl_ctx_alloc -- there is an exit there.
> 
> Is it required to install the atexit handler before alloc()?
> To me it looks like atexit(xl_ctx_free) should be done right after, or inside, xl_ctx_alloc().

Note that multiple calls to atexit creates a chain of functions to
call, not overriding what is previously registered.

The xl_ctx_alloc function is also called in postfork. I think if you
move atexit call into xl_ctx_alloc you will end up registering
xl_ctx_free more than once, which is wrong.

The other option -- to call atexit right after -- looks plausible to me.

Wei.

> 
> Olaf



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xl: free bitmaps on exit

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 233 bytes --]

Am Tue, 27 Nov 2018 17:19:06 +0000
schrieb Wei Liu <wei.liu2@citrix.com>:

> The other option -- to call atexit right after -- looks plausible to me.

After looking through main(), moving the atexit() seems safe to me.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel