Hi Philip,

On Tue, Apr 14, 2015 at 10:29:33AM -0700, Philip P. Moltmann wrote:
> Before this patch the slow memory transfer would cause the destination VM to
> have internal swapping until all memory is transferred. Now the memory is
> transferred fast enough so that the destination VM does not swap. The balloon
> loop already yields to the rest of the system, hence the balloon thread
> should not monopolize a cpu.
> 
> Testing Done: quickly ballooned a lot of pages while wathing if there are any
> perceived hickups (periods of non-responsiveness) in the execution of the
> linux VM. No such hickups were seen.

What happens if you run this new driver on an older hypervisor that does
not support batched operations?

> 
> Signed-off-by: Xavier Deguillard <xdeguill...@vmware.com>
> ---
>  drivers/misc/vmw_balloon.c | 66 
> +++++++++++-----------------------------------
>  1 file changed, 15 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index 6eaf7f7..a5e1980 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -46,7 +46,7 @@
>  
>  MODULE_AUTHOR("VMware, Inc.");
>  MODULE_DESCRIPTION("VMware Memory Control (Balloon) Driver");
> -MODULE_VERSION("1.3.3.0-k");
> +MODULE_VERSION("1.3.4.0-k");
>  MODULE_ALIAS("dmi:*:svnVMware*:*");
>  MODULE_ALIAS("vmware_vmmemctl");
>  MODULE_LICENSE("GPL");
> @@ -57,12 +57,6 @@ MODULE_LICENSE("GPL");
>   */
>  
>  /*
> - * Rate of allocating memory when there is no memory pressure
> - * (driver performs non-sleeping allocations).
> - */
> -#define VMW_BALLOON_NOSLEEP_ALLOC_MAX        16384U
> -
> -/*
>   * Rates of memory allocaton when guest experiences memory pressure
>   * (driver performs sleeping allocations).
>   */
> @@ -71,13 +65,6 @@ MODULE_LICENSE("GPL");
>  #define VMW_BALLOON_RATE_ALLOC_INC   16U
>  
>  /*
> - * Rates for releasing pages while deflating balloon.
> - */
> -#define VMW_BALLOON_RATE_FREE_MIN    512U
> -#define VMW_BALLOON_RATE_FREE_MAX    16384U
> -#define VMW_BALLOON_RATE_FREE_INC    16U
> -
> -/*
>   * When guest is under memory pressure, use a reduced page allocation
>   * rate for next several cycles.
>   */
> @@ -99,9 +86,6 @@ MODULE_LICENSE("GPL");
>   */
>  #define VMW_PAGE_ALLOC_CANSLEEP              (GFP_HIGHUSER)
>  
> -/* Maximum number of page allocations without yielding processor */
> -#define VMW_BALLOON_YIELD_THRESHOLD  1024
> -
>  /* Maximum number of refused pages we accumulate during inflation cycle */
>  #define VMW_BALLOON_MAX_REFUSED              16
>  
> @@ -278,7 +262,6 @@ struct vmballoon {
>  
>       /* adjustment rates (pages per second) */
>       unsigned int rate_alloc;
> -     unsigned int rate_free;
>  
>       /* slowdown page allocations for next few cycles */
>       unsigned int slow_allocation_cycles;
> @@ -502,18 +485,13 @@ static bool vmballoon_send_batched_unlock(struct 
> vmballoon *b,
>  static void vmballoon_pop(struct vmballoon *b)
>  {
>       struct page *page, *next;
> -     unsigned int count = 0;
>  
>       list_for_each_entry_safe(page, next, &b->pages, lru) {
>               list_del(&page->lru);
>               __free_page(page);
>               STATS_INC(b->stats.free);
>               b->size--;
> -
> -             if (++count >= b->rate_free) {
> -                     count = 0;
> -                     cond_resched();
> -             }
> +             cond_resched();
>       }
>  
>       if ((b->capabilities & VMW_BALLOON_BATCHED_CMDS) != 0) {
> @@ -742,13 +720,13 @@ static void vmballoon_inflate(struct vmballoon *b)
>        * Start with no sleep allocation rate which may be higher
>        * than sleeping allocation rate.
>        */
> -     rate = b->slow_allocation_cycles ?
> -                     b->rate_alloc : VMW_BALLOON_NOSLEEP_ALLOC_MAX;
> +     rate = b->slow_allocation_cycles ? b->rate_alloc : -1;
>  
>       pr_debug("%s - goal: %d, no-sleep rate: %d, sleep rate: %d\n",
>                __func__, b->target - b->size, rate, b->rate_alloc);
>  
> -     while (b->size < b->target && num_pages < b->target - b->size) {
> +     while (!b->reset_required &&
> +             b->size < b->target && num_pages < b->target - b->size) {
>               struct page *page;
>  
>               if (flags == VMW_PAGE_ALLOC_NOSLEEP)
> @@ -798,12 +776,9 @@ static void vmballoon_inflate(struct vmballoon *b)
>                               break;
>               }
>  
> -             if (++allocations > VMW_BALLOON_YIELD_THRESHOLD) {
> -                     cond_resched();
> -                     allocations = 0;
> -             }
> +             cond_resched();
>  
> -             if (allocations >= rate) {
> +             if (rate != -1 && allocations >= rate) {
>                       /* We allocated enough pages, let's take a break. */

Why don't you make the rate UINT_MAX when doing fast allocations, then
you would not need to treat -1 as a special case.

>                       break;
>               }
> @@ -837,36 +812,29 @@ static void vmballoon_deflate(struct vmballoon *b)
>       unsigned int num_pages = 0;
>       int error;
>  
> -     pr_debug("%s - size: %d, target %d, rate: %d\n", __func__, b->size,
> -                                             b->target, b->rate_free);
> +     pr_debug("%s - size: %d, target %d\n", __func__, b->size, b->target);
>  
>       /* free pages to reach target */
>       list_for_each_entry_safe(page, next, &b->pages, lru) {
>               list_del(&page->lru);
>               b->ops->add_page(b, num_pages++, page);
>  
> +

Seems line unintended whitespace addition.

>               if (num_pages == b->batch_max_pages) {
>                       error = b->ops->unlock(b, num_pages, &b->target);
>                       num_pages = 0;
> -                     if (error) {
> -                             /* quickly decrease rate in case of error */
> -                             b->rate_free = max(b->rate_free / 2,
> -                                             VMW_BALLOON_RATE_FREE_MIN);
> +                     if (error)
>                               return;
> -                     }
>               }
>  
> -             if (++i >= b->size - b->target)
> +             if (b->reset_required || ++i >= b->size - b->target)
>                       break;
> +
> +             cond_resched();
>       }
>  
>       if (num_pages > 0)
>               b->ops->unlock(b, num_pages, &b->target);
> -
> -     /* slowly increase rate if there were no errors */
> -     if (error == 0)
> -             b->rate_free = min(b->rate_free + VMW_BALLOON_RATE_FREE_INC,
> -                                VMW_BALLOON_RATE_FREE_MAX);
>  }
>  
>  static const struct vmballoon_ops vmballoon_basic_ops = {
> @@ -992,11 +960,8 @@ static int vmballoon_debug_show(struct seq_file *f, void 
> *offset)
>  
>       /* format rate info */
>       seq_printf(f,
> -                "rateNoSleepAlloc:   %8d pages/sec\n"
> -                "rateSleepAlloc:     %8d pages/sec\n"
> -                "rateFree:           %8d pages/sec\n",
> -                VMW_BALLOON_NOSLEEP_ALLOC_MAX,
> -                b->rate_alloc, b->rate_free);
> +                "rateSleepAlloc:     %8d pages/sec\n",
> +                b->rate_alloc);
>  
>       seq_printf(f,
>                  "\n"
> @@ -1087,7 +1052,6 @@ static int __init vmballoon_init(void)
>  
>       /* initialize rates */
>       balloon.rate_alloc = VMW_BALLOON_RATE_ALLOC_MAX;
> -     balloon.rate_free = VMW_BALLOON_RATE_FREE_MAX;
>  
>       INIT_DELAYED_WORK(&balloon.dwork, vmballoon_work);

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to