From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH] xl create: endless loop Date: Wed, 10 Nov 2010 11:34:50 +0100 Message-ID: <201011101134.50776.Christoph.Egger@amd.com> References: <201010181450.36050.Christoph.Egger@amd.com> <201011031712.48007.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Stefano Stabellini Cc: Ian, "xen-devel@lists.xensource.com" , Jackson List-Id: xen-devel@lists.xenproject.org On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote: > On Wed, 3 Nov 2010, Christoph Egger wrote: > > No, this patch has no effect for me. > > In libxl__fill_dom0_memory_info(), the code path goes that way: > > > > t = xs_transaction_start(ctx->xsh); > > > > target = libxl__xs_read(gc, t, target_path); > > if (target) { <-- target contains "5" > > *target_memkb = strtoul(target, &endptr, 10); > > if (*endptr != '\0') { <-- *endptr contains '\0' > > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > > "invalid memory target %s from %s\n", target, > > target_path); > > rc = ERROR_FAIL; > > goto out; > > } > > rc = 0; > > goto out; <-- take this jump with rc being 0 > > } > > The problem you are having is that somebody in your system is setting a > target for dom0 without setting freemem-slack. Are you still running > xend at boot? Yes, I do. Xen is booted with dom0_mem. Dom0 has autoballooning disabled in the kernel. > Currently libxl__fill_dom0_memory_info assumes that both values are set > initially at the same time (by libxl__fill_dom0_memory_info). > This patch should fix the issue, I would appreciate if you could test > it. > > --- > > > libxl: do not assume target and freemem-slack are written at the same time > > Signed-off-by: Stefano Stabellini > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 > +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( > int rc; > libxl_dominfo info; > libxl_physinfo physinfo; > - char *target = NULL, *endptr = NULL; > + char *target = NULL, *staticmax = NULL, *freememslack = NULL, *endptr > = NULL; char *target_path = "/local/domain/0/memory/target"; > char *max_path = "/local/domain/0/memory/static-max"; > char *free_mem_slack_path = "/local/domain/0/memory/freemem-slack"; > xs_transaction_t t; > libxl_ctx *ctx = libxl__gc_owner(gc); > - uint32_t free_mem_slack = 0; > + uint32_t free_mem_slack_kb = 0; > > retry_transaction: > t = xs_transaction_start(ctx->xsh); > > target = libxl__xs_read(gc, t, target_path); > + staticmax = libxl__xs_read(gc, t, target_path); > + freememslack = libxl__xs_read(gc, t, target_path); > + if (target && staticmax && freememslack) { > + rc = 0; > + goto out; > + } *target, *staticmax and *freememslack contain the value "5". So with this patch, rc always returns 0 from there. Christoph > + > if (target) { > *target_memkb = strtoul(target, &endptr, 10); > if (*endptr != '\0') { > @@ -2799,38 +2806,43 @@ retry_transaction: > rc = ERROR_FAIL; > goto out; > } > - rc = 0; > - goto out; > } > > rc = libxl_domain_info(ctx, &info, 0); > if (rc < 0) > - return rc; > + goto out; > > rc = libxl_get_physinfo(ctx, &physinfo); > if (rc < 0) > - return rc; > - > - libxl__xs_write(gc, t, target_path, "%"PRIu32, > - (uint32_t) info.current_memkb); > - libxl__xs_write(gc, t, max_path, "%"PRIu32, > - (uint32_t) info.max_memkb); > - > - free_mem_slack = (uint32_t) (PAGE_TO_MEMKB(physinfo.total_pages) - > - info.current_memkb); > - /* From empirical measurements the free_mem_slack shouldn't be more > - * than 15% of the total memory present on the system. */ > - if (free_mem_slack > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) > - free_mem_slack = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; > - libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, > free_mem_slack); - > - *target_memkb = (uint32_t) info.current_memkb; > + goto out; > + > + if (target == NULL) { > + libxl__xs_write(gc, t, target_path, "%"PRIu32, > + (uint32_t) info.current_memkb); > + *target_memkb = (uint32_t) info.current_memkb; > + } > + if (staticmax == NULL) > + libxl__xs_write(gc, t, max_path, "%"PRIu32, > + (uint32_t) info.max_memkb); > + > + if (freememslack == NULL) { > + free_mem_slack_kb = (uint32_t) > (PAGE_TO_MEMKB(physinfo.total_pages) - + > info.current_memkb); > + /* From empirical measurements the free_mem_slack shouldn't be > more + * than 15% of the total memory present on the system. */ + > if (free_mem_slack_kb > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) + > free_mem_slack_kb = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; + > libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, > free_mem_slack_kb); + } > rc = 0; > > out: > - if (!xs_transaction_end(ctx->xsh, t, 0)) > + if (!xs_transaction_end(ctx->xsh, t, 0)) { > if (errno == EAGAIN) > goto retry_transaction; > + else > + rc = ERROR_FAIL; > + } > > > return rc; -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632