[Xen-devel] RE: [PATCH] Xenoprof passive domain support

["Santos, Jose Renato G" <joserenato.santos@xxxxxx>]

Xiaowei,

Thanks for the patch.
I took a first look in the patch and have a few comments.

First, I think there is a concurrency problem that needs to
fixed. If dom0 has more than one VCPU, then 2 VCPUs can
access the same passive buffer at the same time. This
should not be allowed, since the buffer is not
protected by any lock. We should make sure that any
passive domain buffer is accessed by one and only one
dom0 VCPU. A simple approach would be to have only
VCPU 0 (in dom0) to handle all the passive domain
samples. However, this may cause an 
imbalance on the speed that oprofile CPU buffers are filled.
Probably the cpu buffer for VCPU 0 would fill much
earlier and cause more frequent flushes to the event buffer,
increasing the overhead.
I would prefer if we could distribute the passive
buffers to all VCPUs in dom0. I will think more about
this during the weekend and make additional comments,
earlier next week.

Second, I see that you reused most of the code that
was used in Xen 2.0, when passive domains was supported
for non SMP guests. 
John Levon (cc'ed on this message) has made some comments
on that code which I think we should address right now. 
More specifically, he suggested that we change the way
domain switches are represented in the CPU buffer. 
Please look at his comment below. 

I suggest that we address these 2 issues first and
then I can look more carefully at the rest of the code.
Please CC John Levon, on you next messages

Thanks

Renato

Old comments by John Levon on passive domain code:
=============================================================
I'm uncomfortable with the chosen domain switching encoding. It's pretty
confusing to follow all the escaping done already, and you're making it
more complicated. Why can't you use a normal ESCAPE_CODE? I.e. the
buffer would have

   EIP          Event
---------------------------------
0. ESCAPE_CODE  DOMAIN_SWITCH
1. domain ID
2. EIP          Event

Sure, you lose an entry (1) in size but it means simpler code, and less
i-cache hurt.

Why is the state not persistent until the next escape code? i.e. it's
only for one sample.

======================================================================
>> -----Original Message-----
>> From: Yang, Xiaowei [mailto:xiaowei.yang@xxxxxxxxx] 
>> Sent: Friday, April 28, 2006 12:59 AM
>> To: Santos, Jose Renato G
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
>> Subject: [PATCH] Xenoprof passive domain support
>> 
>> Hi Renato,
>> This patch is to add Xenoprof passive domain support in SMP 
>> environment. Basically: 
>> - It allocates per vcpu buffers for passive domain and maps 
>> them into primary domain's space. 
>> - When primary domain gets sampled and triggers virq, its 
>> kernel module will handle passive domain' samples besides 
>> its owns. There is potential buffer overflow if passive is 
>> very busy while primary is idle. The simple workaround is to 
>> allocate a rather big buffer for passive. For now it works 
>> fine. A better way is to notify primary when passive buffer 
>> exceeds the threshold.
>> - When oprofile gets passive domain samples, it splits them 
>> in to 3 parts by PC ranges pre-definition: 
>> xen/kernel/application and maps the first two into 
>> functions. The oprofile version is a little old, for the 
>> patch was made last August. 
>> 
>> The main usage is to use Xenoprof to profile HVM domain as 
>> passive domain, which provides enough information for low 
>> level performance tuning while no change in HVM is needed.
>> 
>> Thanks,
>> Xiaowei
>> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel