Re: [PATCH 2/4] libxl_wait_for_memory_target: wait as long as dom0 is making progress

[Jim Fehlig <jfehlig@suse.com>]

Stefano Stabellini wrote:
> Decrement wait_secs only if dom0 is making no progress toward reaching
> the balloon target, otherwise loop again for free.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Tested-by: Mike Latimer <mlatimer@suse.com>
> ---
>  tools/libxl/libxl.c      |   29 ++++++++++++++++++++++-------
>  tools/libxl/xl_cmdimpl.c |    4 ++--
>  2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 088786e..648a227 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4959,26 +4959,41 @@ int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs)
>   

Would be nice to have a comment clarifying the semantics of wait_secs,
otherwise callers might assume it is the wait time to reach the memory
target vs the wait time if no ballooning progress is being made.

Regards,
Jim

>  {
>      int rc = 0;
>      uint32_t target_memkb = 0;
> +    uint64_t current_memkb, prev_memkb;
>      libxl_dominfo info;
>  
> +    rc = libxl_get_memory_target(ctx, domid, &target_memkb);
> +    if (rc < 0)
> +        return rc;
> +
>      libxl_dominfo_init(&info);
> +    prev_memkb = UINT64_MAX;
>  
>      do {
> -        wait_secs--;
>          sleep(1);
>  
> -        rc = libxl_get_memory_target(ctx, domid, &target_memkb);
> -        if (rc < 0)
> -            goto out;
> -
>          libxl_dominfo_dispose(&info);
>          libxl_dominfo_init(&info);
>          rc = libxl_domain_info(ctx, &info, domid);
>          if (rc < 0)
>              goto out;
> -    } while (wait_secs > 0 && (info.current_memkb + info.outstanding_memkb) > target_memkb);
>  
> -    if ((info.current_memkb + info.outstanding_memkb) <= target_memkb)
> +        current_memkb = info.current_memkb + info.outstanding_memkb;
> +
> +        if (current_memkb > prev_memkb)
> +        {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        else if (current_memkb == prev_memkb)
> +            wait_secs--;
> +        /* if current_memkb < prev_memkb loop for free as progress has
> +         * been made */
> +
> +        prev_memkb = current_memkb;
> +    } while (wait_secs > 0 && current_memkb > target_memkb);
> +
> +    if (current_memkb <= target_memkb)
>          rc = 0;
>      else
>          rc = ERROR_FAIL;
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index f4c4122..2dc7574 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2226,8 +2226,8 @@ static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
>          else if (rc != ERROR_NOMEM)
>              return rc;
>  
> -        /* the memory target has been reached but the free memory is still
> -         * not enough: loop over again */
> +        /* wait until dom0 reaches its target, as long as we are making
> +         * progress */
>          rc = libxl_wait_for_memory_target(ctx, 0, 1);
>          if (rc < 0)
>              return rc;
>