From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH] xl create: endless loop Date: Tue, 9 Nov 2010 18:08:14 +0000 Message-ID: References: <201010181450.36050.Christoph.Egger@amd.com> <201010191031.32340.Christoph.Egger@amd.com> <201011031712.48007.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Return-path: In-Reply-To: <201011031712.48007.Christoph.Egger@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Christoph Egger Cc: "xen-devel@lists.xensource.com" , Ian Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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? 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; + } + 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;