[PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack

[PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack

[Paul Durrant]

From: Paul Durrant <pdurrant@amazon.com>

Currently an array of 1024 xc_domaininfo_t is declared on stack. That alone
consumes ~112k. Since libxl_domain_info() creates a new gc this patch simply
uses it to allocate the array instead.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>

This is small and IMO it would be nice to have this in 4.14 but I'd like an
opinion from a maintainer too.
---
 tools/libxl/libxl_domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index fef2cd4e13..c07ec8fd3a 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -314,11 +314,13 @@ libxl_dominfo * libxl_list_domain(libxl_ctx *ctx, int *nb_domain_out)
 {
     libxl_dominfo *ptr = NULL;
     int i, ret;
-    xc_domaininfo_t info[1024];
+    xc_domaininfo_t *info;
     int size = 0;
     uint32_t domid = 0;
     GC_INIT(ctx);
 
+    info = libxl__calloc(gc, 1024, sizeof(*info));
+
     while ((ret = xc_domain_getinfolist(ctx->xch, domid, 1024, info)) > 0) {
         ptr = libxl__realloc(NOGC, ptr, (size + ret) * sizeof(libxl_dominfo));
         for (i = 0; i < ret; i++) {
-- 
2.17.1

Re: [PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack

[Ian Jackson]

Paul Durrant writes ("[PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack"):
> From: Paul Durrant <pdurrant@amazon.com>
> 
> Currently an array of 1024 xc_domaininfo_t is declared on stack. That alone
> consumes ~112k.

Wow.

> Since libxl_domain_info() creates a new gc this patch simply
> uses it to allocate the array instead.

Thanks.

> +    info = libxl__calloc(gc, 1024, sizeof(*info));

Wy not GCNEW_ARRAY ?

That avoids a possible bug with wrong number of * to sizeof (although
in this case you seem to have it right...)

Thanks,
Ian.

RE: [PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack

[Paul Durrant]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 28 May 2020 15:57
> To: Paul Durrant <paul@xen.org>
> Cc: xen-devel@lists.xenproject.org; Paul Durrant <pdurrant@amazon.com>; Wei Liu <wl@xen.org>; Anthony
> Perard <anthony.perard@citrix.com>
> Subject: Re: [PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack
> 
> Paul Durrant writes ("[PATCH] libxl: stop libxl_domain_info() consuming massive amounts of stack"):
> > From: Paul Durrant <pdurrant@amazon.com>
> >
> > Currently an array of 1024 xc_domaininfo_t is declared on stack. That alone
> > consumes ~112k.
> 
> Wow.
> 
> > Since libxl_domain_info() creates a new gc this patch simply
> > uses it to allocate the array instead.
> 
> Thanks.
> 
> > +    info = libxl__calloc(gc, 1024, sizeof(*info));
> 
> Wy not GCNEW_ARRAY ?

'Cos I didn't know about that one :-)

> 
> That avoids a possible bug with wrong number of * to sizeof (although
> in this case you seem to have it right...)
> 

Cool. I'll send a v2 in mo'.

  Paul

> Thanks,
> Ian.