Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit
Am Tue, 27 Nov 2018 17:19:06 + schrieb Wei Liu : > The other option -- to call atexit right after -- looks plausible to me. After looking through main(), moving the atexit() seems safe to me. Olaf pgpYhWjmpbrZl.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit
On Tue, Nov 27, 2018 at 06:09:33PM +0100, Olaf Hering wrote: > Am Tue, 27 Nov 2018 16:55:38 + > schrieb Wei Liu : > > > 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: [Xen-devel] [PATCH v2] xl: free bitmaps on exit
Am Tue, 27 Nov 2018 16:55:38 + schrieb Wei Liu : > 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 pgpztdhceiwC0.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xl: free bitmaps on exit
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
> ---
> 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", , 0))
> max_maptrack_frames = l;
>
> -libxl_bitmap_init(_vm_affinity_mask);
> libxl_cpu_bitmap_alloc(ctx, _vm_affinity_mask, 0);
> -libxl_bitmap_init(_hvm_affinity_mask);
> libxl_cpu_bitmap_alloc(ctx, _hvm_affinity_mask, 0);
> -libxl_bitmap_init(_pv_affinity_mask);
> libxl_cpu_bitmap_alloc(ctx, _pv_affinity_mask, 0);
>
> if (!xlu_cfg_get_string (config, "vm.cpumask", , 0))
> @@ -323,11 +320,17 @@ void xl_ctx_alloc(void) {
> exit(1);
> }
>
> +libxl_bitmap_init(_vm_affinity_mask);
> +libxl_bitmap_init(_hvm_affinity_mask);
> +libxl_bitmap_init(_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: [Xen-devel] [PATCH v2] xl: free bitmaps on exit
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 Acked-by: Wei Liu Thanks for fixing this. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xl: free bitmaps on exit
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
---
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", , 0))
max_maptrack_frames = l;
-libxl_bitmap_init(_vm_affinity_mask);
libxl_cpu_bitmap_alloc(ctx, _vm_affinity_mask, 0);
-libxl_bitmap_init(_hvm_affinity_mask);
libxl_cpu_bitmap_alloc(ctx, _hvm_affinity_mask, 0);
-libxl_bitmap_init(_pv_affinity_mask);
libxl_cpu_bitmap_alloc(ctx, _pv_affinity_mask, 0);
if (!xlu_cfg_get_string (config, "vm.cpumask", , 0))
@@ -323,11 +320,17 @@ void xl_ctx_alloc(void) {
exit(1);
}
+libxl_bitmap_init(_vm_affinity_mask);
+libxl_bitmap_init(_hvm_affinity_mask);
+libxl_bitmap_init(_pv_affinity_mask);
libxl_childproc_setmode(ctx, _hooks, 0);
}
static void xl_ctx_free(void)
{
+libxl_bitmap_dispose(_pv_affinity_mask);
+libxl_bitmap_dispose(_hvm_affinity_mask);
+libxl_bitmap_dispose(_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
