[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Xen 4.3 + tmem = Xen BUG at domain_page.c:143



>>> On 12.06.13 at 17:27, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> On 12/06/2013 16:11, "Keir Fraser" <keir.xen@xxxxxxxxx> wrote:
> 
>> On 12/06/2013 13:12, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> 
>>>> What we see above is that this failure path succeeds three times, but
>>>> fails the fourth time: there are, in fact, no zero entries after the
>>>> garbage clean-up; however, because "inuse" is 32-bit (effectively) and
>>>> "accum" is 64-bit, ~inuse always has bits 32-63 set, and so will
>>>> always return true and never fall back to the "something else"
>>> 
>>> Right, that's what occurred to me too yesterday, but the again
>>> I knew I had seen this code path executed. Now that I look again,
>>> I think I understand why: All of my Dom0-s and typical DomU-s
>>> have a vCPU count divisible by 4, and with MAPCACHE_VCPU_ENTRIES
>>> being 16, the full unsigned long would always be used.
>> 
>> Why are we so tight on MAPCACHE_VCPU_ENTRIES? Why not say double that number
>> and get rid of the accum and the 'replace a hash entry instead' logic
>> instead? We never used to have it, and it's kind of extra complication and a
>> bit gross.
> 
> Could even pull MAPHASH_ENTRIES into config.h and explicitly add it to
> MAPCACHE_VCPU_ENTRIES. That would be neat would it not? And I believe we
> have space in the mapcache's perdomain slot to enlarge MAPCACHE_VCPU_ENTRIES
> in this way (i.e., from 16 to 24).

Yes, this would an option, but I very much dislike pulling further
stuff not belonging there into config.h - this define really best
lives side by side with its related definitions.

While I just checked the code and think that this not being a power
of two should be fine, I'd nevertheless be careful with such a
change.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.