What's the status of this? As I had pointed out before, this change alone
(without a tools side adjustment) will not work. And the question still
wasn't answered clearly whether the full amount of memory will really be
needed at this point (i.e. before the second stage of initialization, which
happens after the full size ballooning has taken place).
Of course, the patch as it was presented here would at least restore
old behavior for guests with not too many vCPU-s (and get hypervisor
and tools back in sync again), so I'd rather see this patch applied than
nothing further happening at all.
Thanks, Jan
>>> "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx> 14.01.10 08:19 >>>
Keir and Jan,
Ignore the previous mail, the patch in the text is incomplete...
I am working on the issue "pre-reservation of memory for domain creation".
Now I have the following findings.
Currently guest initialization process in xend (XendDomainInfo.py) is:
_constructDomain() --> domain_create() --> domain_max_vcpus() ... -->
_initDomain() --> shadow_mem_control() ...
In domain_create, previously we reserve 1M memory for domain creation (as
described in xend comment), and these memory SHOULD NOT related with vcpu
number. And later, shadow_mem_control() will modify the shadow size to 256
pages per vcpu (also plus some other values related with guest memory size...).
Therefore the C/S 20389 which modifies 1M to 4M to fit more vcpu number is
wrong. I'm sorry for that.
Following is the reason why currently 1M doesn't work for big number vcpus,
as we mentioned, it caused Xen crash.
Each time when sh_set_allocation() is called, it checks whether
shadow_min_acceptable_pages() has been allocated, if not, it will allocate
them. That is to say, it is 128 pages per vcpu. But before we define
d->max_vcpu, guest vcpu hasn't been initialized, so
shadow_min_acceptable_pages() always returns 0. Therefore we only allocated 1M
shadow memory for domain_create, and didn't satisfy 128 pages per vcpu for
alloc_vcpu().
As we know, vcpu allocation is done in the hypercall of
XEN_DOMCTL_max_vcpus. However, at this point we haven't called
shadow_mem_control() and are still using the pre-allocated 1M shadow memory to
allocate so many vcpus. So it should be a BUG. Therefore when vcpu number
increases, 1M is not enough and causes Xen crash. C/S 20389 exposes this issue.
So I think the right process should be, after d->max_vcpu is set and before
alloc_vcpu(), we should call sh_set_allocation() to satisfy 128 pages per vcpu.
The following patch does this work. Is it work for you? Thanks!
Best Regards,
-- Dongxiao
Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
diff -r 13d4e78ede97 xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c Wed Jan 13 08:33:34 2010 +0000
+++ b/xen/arch/x86/mm/shadow/common.c Thu Jan 14 14:02:23 2010 +0800
@@ -41,6 +41,9 @@
DEFINE_PER_CPU(uint32_t,trace_shadow_path_flags);
+static unsigned int sh_set_allocation(struct domain *d,
+ unsigned int pages,
+ int *preempted);
/* Set up the shadow-specific parts of a domain struct at start of day.
* Called for every domain from arch_domain_create() */
void shadow_domain_init(struct domain *d, unsigned int domcr_flags)
@@ -82,6 +85,12 @@ void shadow_vcpu_init(struct vcpu *v)
}
#endif
+ if ( !is_idle_domain(v->domain) )
+ {
+ shadow_lock(v->domain);
+ sh_set_allocation(v->domain, 128, NULL);
+ shadow_unlock(v->domain);
+ }
v->arch.paging.mode = &SHADOW_INTERNAL_NAME(sh_paging_mode, 3);
}
@@ -3099,7 +3108,7 @@ int shadow_enable(struct domain *d, u32
{
unsigned int r;
shadow_lock(d);
- r = sh_set_allocation(d, 1024, NULL); /* Use at least 4MB */
+ r = sh_set_allocation(d, 256, NULL); /* Use at least 1MB */
if ( r != 0 )
{
sh_set_allocation(d, 0, NULL);
Jan Beulich wrote:
>>>> "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx> 13.01.10 03:34 >>>
>> If we didn't add this change, as Keir said, Xen will crash during
>> destruction of partially-created domain.
>
> If this indeed is reproducible, I think it should be fixed.
>
>> However I didn't noticed the toolstack and
>> shadow_min_acceptable_pages() side at that time...
>> For now, should we adjust the shadow pre-alloc size to match
>> shadow_min_acceptable_pages() and modify toolstack accordingly?
>
> I would say so, just with the problem that I can't reliable say what
> "accordingly" here would be (and hence I can't craft a patch I can
> guarantee will work at least in most of the cases).
>
> And as said before, I'm also not convinced that using the maximum
> possible number of vCPU-s for this initial calculation is really the
> right thing to do.
>
> Jan
Best Regards,
-- Dongxiao
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|