On 3/6/19 7:35 PM, Alexander Duyck wrote:
> Here are some changes I made to your patch in order to address the sizing
> issue I called out. You may want to try testing with this patch applied to
> your QEMU as I am finding it is making a signficant difference. It has cut
> the test time for the 32G memhog test I called out earlier in half.
Thanks, I will try this out.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  hw/virtio/virtio-balloon.c |   28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index d2cf66ada3c0..3ca6b1c6d511 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -285,7 +285,7 @@ static void balloon_stats_set_poll_interval(Object *obj, Visitor *v,
>      balloon_stats_change_timer(s, 0);
>  }
>  
> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> +static void *gpa2hva(MemoryRegion **p_mr, unsigned long *size, hwaddr addr, Error **errp)
>  {
>      MemoryRegionSection mrs = memory_region_find(get_system_memory(),
>                                                   addr, 1);
> @@ -302,6 +302,7 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
>      }
>  
>      *p_mr = mrs.mr;
> +    *size = mrs.mr->size - mrs.offset_within_region;
>      return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
>  }
>  
> @@ -313,30 +314,35 @@ void page_hinting_request(uint64_t addr, uint32_t len)
>      struct guest_pages *guest_obj;
>      int i = 0;
>      void *hvaddr_to_free;
> -    unsigned long pfn, pfn_end;
>      uint64_t gpaddr_to_free;
> -    void * temp_addr = gpa2hva(&mr, addr, &local_err);
> +    unsigned long madv_size, size;
> +    void * temp_addr = gpa2hva(&mr, &madv_size, addr, &local_err);
>  
>      if (local_err) {
>          error_report_err(local_err);
>          return;
>      }
> +    if (madv_size < sizeof(*guest_obj)) {
> +	printf("\nBad guest object ptr\n");
> +	return;
> +    }
>      guest_obj = temp_addr;
>      while (i < len) {
> -        pfn = guest_obj[i].pfn;
> -	pfn_end = guest_obj[i].pfn + (1 << guest_obj[i].order) - 1;
> -	trace_virtio_balloon_hinting_request(pfn,(1 << guest_obj[i].order));
> -	while (pfn <= pfn_end) {
> -	        gpaddr_to_free = pfn << VIRTIO_BALLOON_PFN_SHIFT;
> -	        hvaddr_to_free = gpa2hva(&mr, gpaddr_to_free, &local_err);
> +        gpaddr_to_free = guest_obj[i].pfn << VIRTIO_BALLOON_PFN_SHIFT;
> +	size = (1 << VIRTIO_BALLOON_PFN_SHIFT) << guest_obj[i].order;
> +	while (size) {
> +	        hvaddr_to_free = gpa2hva(&mr, &madv_size, gpaddr_to_free, &local_err);
>  	        if (local_err) {
>  			error_report_err(local_err);
>  		        return;
>  		}
> -		ret = qemu_madvise((void *)hvaddr_to_free, 4096, QEMU_MADV_DONTNEED);
> +		if (size < madv_size)
> +			madv_size = size;
> +		ret = qemu_madvise((void *)hvaddr_to_free, madv_size, QEMU_MADV_DONTNEED);
>  		if (ret == -1)
>  		    printf("\n%d:%s Error: Madvise failed with error:%d\n", __LINE__, __func__, ret);
> -		pfn++;
> +		gpaddr_to_free += madv_size;
> +		size -= madv_size;
>  	}
>  	i++;
>      }
>
-- 
Regards
Nitesh