[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel][PATCH] xl create: endless loop



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 <stefano.stabellini@xxxxxxxxxxxxx>
>
> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.