Re: [Xen-devel] [Qemu-devel] frequently ballooning results in qemu exit

[Hanweidong <hanweidong@xxxxxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx]
> Sent: 2013年3月29日 20:37
> To: Hanweidong
> Cc: Tim (Xen.org); George Dunlap; Andrew Cooper; Yanqiangjun; qemu-
> devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; Gonglei (Arei); Anthony
> Perard; Wangzhenguo
> Subject: Re: [Qemu-devel] [Xen-devel] frequently ballooning results in
> qemu exit
> 
> On Mon, 25 Mar 2013, Hanweidong wrote:
> > We fixed this issue by below patch which computed the correct size
> for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call
> xen_map_cache() with size is 0 if the requested address is in the RAM.
> Then xen_map_cache() will pass the size 0 to test_bits() for checking
> if the corresponding pfn was mapped in cache. But test_bits() will
> always return 1 when size is 0 without any bit testing. Actually, for
> this case, test_bits should check one bit. So this patch introduced a
> __test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE,
> then test_bits can work correctly with __test_bit_size >> XC_PAGE_SHIFT
> as its size.
> >
> > Signed-off-by: Zhenguo Wang <wangzhenguo@xxxxxxxxxx>
> > Signed-off-by: Weidong Han <hanweidong@xxxxxxxxxx>
> 
> Thanks for the patch and for debugging this difficult problem.
> The reality is that size is never actually 0: when qemu_get_ram_ptr
> calls xen_map_cache with size 0, it actually means "map until the end
> of
> the page". As a consequence test_bits should always test at least 1 bit,
> like you wrote.

Yes, for the case of size is 0, we can just simply set __test_bit_size 1. But 
for size > 0, I think set __test_bit_size to size >> XC_PAGE_SHIFT is 
incorrect. If size is not multiple of XC_PAGE_SIZE, then the part of (size % 
XC_PAGE_SIZE) also needs to test 1 bit. For example size is XC_PAGE_SIZE + 1, 
then it needs to test 2 bits, but size >> XC_PAGE_SHIFT is only 1. 

-weidong

> 
> I tried to simplify your patch a bit, does this one work for you?
> 
> 
> diff --git a/xen-mapcache.c b/xen-mapcache.c
> index dc6d1fa..b03b373 100644
> --- a/xen-mapcache.c
> +++ b/xen-mapcache.c
> @@ -202,6 +202,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr
> size,
>      hwaddr address_index;
>      hwaddr address_offset;
>      hwaddr __size = size;
> +    hwaddr __test_bit_size = size >> XC_PAGE_SHIFT;
>      bool translated = false;
> 
>  tryagain:
> @@ -224,12 +225,16 @@ tryagain:
>      } else {
>          __size = MCACHE_BUCKET_SIZE;
>      }
> +    /* always test at least one page */
> +    if (!__test_bit_size) {
> +        __test_bit_size = 1UL;
> +    }
> 
>      entry = &mapcache->entry[address_index % mapcache->nr_buckets];
> 
>      while (entry && entry->lock && entry->vaddr_base &&
>              (entry->paddr_index != address_index || entry->size !=
> __size ||
> -             !test_bits(address_offset >> XC_PAGE_SHIFT, size >>
> XC_PAGE_SHIFT,
> +             !test_bits(address_offset >> XC_PAGE_SHIFT,
> __test_bit_size,
>                   entry->valid_mapping))) {
>          pentry = entry;
>          entry = entry->next;
> @@ -241,13 +246,13 @@ tryagain:
>      } else if (!entry->lock) {
>          if (!entry->vaddr_base || entry->paddr_index != address_index
> ||
>                  entry->size != __size ||
> -                !test_bits(address_offset >> XC_PAGE_SHIFT, size >>
> XC_PAGE_SHIFT,
> +                !test_bits(address_offset >> XC_PAGE_SHIFT,
> __test_bit_size,
>                      entry->valid_mapping)) {
>              xen_remap_bucket(entry, __size, address_index);
>          }
>      }
> 
> -    if(!test_bits(address_offset >> XC_PAGE_SHIFT, size >>
> XC_PAGE_SHIFT,
> +    if(!test_bits(address_offset >> XC_PAGE_SHIFT, __test_bit_size,
>                  entry->valid_mapping)) {
>          mapcache->last_address_index = -1;
>          if (!translated && mapcache->phys_offset_to_gaddr) {
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel