From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [Qemu-devel] frequently ballooning results in qemu exit Date: Fri, 29 Mar 2013 12:37:29 +0000 Message-ID: References: <5141A8B0.4050305@citrix.com> <20130314143403.GB5174@ocelot.phlegethon.org> <20130321121547.GB12338@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Hanweidong Cc: George Dunlap , Andrew Cooper , Yanqiangjun , "Tim (Xen.org)" , "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" , "Gonglei (Arei)" , Anthony Perard , Wangzhenguo List-Id: xen-devel@lists.xenproject.org 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 > Signed-off-by: Weidong Han 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. 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) {