From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86: fix map_domain_page() last resort fallback Date: Thu, 13 Jun 2013 09:06:57 +0100 Message-ID: References: <51B9958E02000078000DDD31@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51B9958E02000078000DDD31@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: George Dunlap , Konrad Rzeszutek Wilk , xen-devel List-Id: xen-devel@lists.xenproject.org On 13/06/2013 08:49, "Jan Beulich" wrote: >>>> On 12.06.13 at 19:27, Keir Fraser wrote: >> On 12/06/2013 16:59, "Jan Beulich" wrote: >> >>> Guests with vCPU count not divisible by 4 have unused bits in the last >>> word of their inuse bitmap, and the garbage collection code therefore >>> would get mislead believing that some entries were actually recoverable >>> for use. >>> >>> Also use an earlier established local variable in mapcache_vcpu_init() >>> instead of re-calculating the value (noticed while investigating the >>> generally better option of setting those overhanging bits once during >>> setup - this didn't work out in a simple enough fashion because the >>> mapping getting established there isn't in the current address space, >>> and hence the bitmap isn't directly accessible there). >>> >>> Reported-by: Konrad Wilk >>> Signed-off-by: Jan Beulich >> >> Whilst I can't argue against this as the obvious bugfix to the existing >> code, I personally object to clawing back hash-table entries at all. The >> size of the per-vcpu hashtable is small, and it should be perfectly possible >> to always allow enough extra entries in the mapcache to always be able to >> allocate an entry even when all vcpu's maphash buckets are in use. >> >> Perhaps this is the right fix for 4.3 at this point, but in that case I am >> quite inclined to simplify this down after 4.3, sidestepping the whole >> issue. > > I won't object undoing this, and moving the MAPHASH_ENTRIES > definition into config.h, but I also won't put my name under it. I think your fix is best for 4.3. Let's get it checked in. Acked-by: Keir Fraser > Jan >