[RESEND][PATCH 0/4] Virtual Machine Time Accounting

[RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Laurent Vivier]

Ingo, please, could you have a look to these patches ?

The aim of these four patches is to introduce Virtual Machine time accounting.

[PATCH 1/4] as recent CPUs introduce a third running state, after "user" and
"system", we need a new field, "guest", in cpustat to store the time used by
the CPU to run virtual CPU. Modify /proc/stat to display this new field.

[PATCH 2/4] like for cpustat, introduce the "gtime" (guest time of the task) and
"cgtime" (guest time of the task children) fields for the
tasks. Modify signal_struct and task_struct. Modify /proc/<pid>/stat to display
these new fields.

[PATCH 3/4] modify account_system_time() to add cputime to cpustat->guest if we
are running a VCPU. We add this cputime to  cpustat->user instead of
cpustat->system because this part of KVM code is in fact user code although it
is executed in the kernel. We duplicate VCPU time between guest and user to
allow an unmodified "top(1)" to display correct value. A modified "top(1)" is
able to display good cpu user time and cpu guest time by subtracting cpu guest
time from cpu user time. Update "gtime" and "cgtime" in signal_struct and
task_struct accordingly.

[PATCH 4/4] Modify KVM to update guest time accounting.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
-- 
------------- Laurent.Vivier@bull.net  --------------
          "Software is hard" - Donald Knuth

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Ingo Molnar]

* Laurent Vivier <Laurent.Vivier@bull.net> wrote:

> Ingo, please, could you have a look to these patches ?
> 
> The aim of these four patches is to introduce Virtual Machine time 
> accounting.
> 
> [PATCH 1/4] as recent CPUs introduce a third running state, after 
> "user" and "system", we need a new field, "guest", in cpustat to store 
> the time used by the CPU to run virtual CPU. Modify /proc/stat to 
> display this new field.

the concept certainly looks sane to me.

The heavy-handed use of #ifdefs uglifies the code to a large degree, but 
this is not a fundamental problem: since basically all distros have KVM 
enabled (and lguest benefits from this too), could you just make all 
this new code unconditional?

	Ingo

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

Ingo Molnar wrote:
> * Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> 
>> Ingo, please, could you have a look to these patches ?
>>
>> The aim of these four patches is to introduce Virtual Machine time 
>> accounting.
>>
>> [PATCH 1/4] as recent CPUs introduce a third running state, after 
>> "user" and "system", we need a new field, "guest", in cpustat to store 
>> the time used by the CPU to run virtual CPU. Modify /proc/stat to 
>> display this new field.
> 
> the concept certainly looks sane to me.
> 
> The heavy-handed use of #ifdefs uglifies the code to a large degree, but 
> this is not a fundamental problem: since basically all distros have KVM 
> enabled (and lguest benefits from this too), could you just make all 
> this new code unconditional?

No problem for me.

Thank you,
Laurent
-- 
------------- Laurent.Vivier@bull.net  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Avi Kivity]

Ingo Molnar wrote:
> * Laurent Vivier <Laurent.Vivier@bull.net> wrote:
>
>   
>> Ingo, please, could you have a look to these patches ?
>>
>> The aim of these four patches is to introduce Virtual Machine time 
>> accounting.
>>
>> [PATCH 1/4] as recent CPUs introduce a third running state, after 
>> "user" and "system", we need a new field, "guest", in cpustat to store 
>> the time used by the CPU to run virtual CPU. Modify /proc/stat to 
>> display this new field.
>>     
>
> the concept certainly looks sane to me.
>
> The heavy-handed use of #ifdefs uglifies the code to a large degree, but 
> this is not a fundamental problem: since basically all distros have KVM 
> enabled (and lguest benefits from this too), could you just make all 
> this new code unconditional?
>   

I imagine the embedded people will complain... perhaps move all the code 
to a #ifdef section above with a full implementation and a stub 
implementation.

-- 
error compiling committee.c: too many arguments to function

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

Avi Kivity wrote:
> Ingo Molnar wrote:
>> * Laurent Vivier <Laurent.Vivier@bull.net> wrote:
>>
>>  
>>> Ingo, please, could you have a look to these patches ?
>>>
>>> The aim of these four patches is to introduce Virtual Machine time
>>> accounting.
>>>
>>> [PATCH 1/4] as recent CPUs introduce a third running state, after
>>> "user" and "system", we need a new field, "guest", in cpustat to
>>> store the time used by the CPU to run virtual CPU. Modify /proc/stat
>>> to display this new field.
>>>     
>>
>> the concept certainly looks sane to me.
>>
>> The heavy-handed use of #ifdefs uglifies the code to a large degree,
>> but this is not a fundamental problem: since basically all distros
>> have KVM enabled (and lguest benefits from this too), could you just
>> make all this new code unconditional?
>>   
> 
> I imagine the embedded people will complain... perhaps move all the code
> to a #ifdef section above with a full implementation and a stub
> implementation.
> 

I'm going to repost patches without #ifdefs for readability.
Then we could discuss if we should introduce #ifdefs and how.

Laurent
-- 
------------- Laurent.Vivier@bull.net  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Alistair John Strachan]

On Monday 10 September 2007 14:08:45 Laurent Vivier wrote:
> Avi Kivity wrote:
> > Ingo Molnar wrote:
> >> * Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> >>> Ingo, please, could you have a look to these patches ?
> >>>
> >>> The aim of these four patches is to introduce Virtual Machine time
> >>> accounting.
> >>>
> >>> [PATCH 1/4] as recent CPUs introduce a third running state, after
> >>> "user" and "system", we need a new field, "guest", in cpustat to
> >>> store the time used by the CPU to run virtual CPU. Modify /proc/stat
> >>> to display this new field.
> >>
> >> the concept certainly looks sane to me.
> >>
> >> The heavy-handed use of #ifdefs uglifies the code to a large degree,
> >> but this is not a fundamental problem: since basically all distros
> >> have KVM enabled (and lguest benefits from this too), could you just
> >> make all this new code unconditional?
> >
> > I imagine the embedded people will complain... perhaps move all the code
> > to a #ifdef section above with a full implementation and a stub
> > implementation.
>
> I'm going to repost patches without #ifdefs for readability.
> Then we could discuss if we should introduce #ifdefs and how.

If you could provide a summary of the size increase from your latest post 
(i.e., size of the kernel before and after) that might lay rest to some 
theoretical complaints.

This doesn't look like enough code to warrant ifdefs.

-- 
Cheers,
Alistair.

137/1 Warrender Park Road, Edinburgh, UK.

Re: [RESEND][PATCH 0/4] Virtual Machine Time Accounting

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]

Alistair John Strachan wrote:
> On Monday 10 September 2007 14:08:45 Laurent Vivier wrote:
>> Avi Kivity wrote:
>>> Ingo Molnar wrote:
>>>> * Laurent Vivier <Laurent.Vivier@bull.net> wrote:
>>>>> Ingo, please, could you have a look to these patches ?
>>>>>
>>>>> The aim of these four patches is to introduce Virtual Machine time
>>>>> accounting.
>>>>>
>>>>> [PATCH 1/4] as recent CPUs introduce a third running state, after
>>>>> "user" and "system", we need a new field, "guest", in cpustat to
>>>>> store the time used by the CPU to run virtual CPU. Modify /proc/stat
>>>>> to display this new field.
>>>> the concept certainly looks sane to me.
>>>>
>>>> The heavy-handed use of #ifdefs uglifies the code to a large degree,
>>>> but this is not a fundamental problem: since basically all distros
>>>> have KVM enabled (and lguest benefits from this too), could you just
>>>> make all this new code unconditional?
>>> I imagine the embedded people will complain... perhaps move all the code
>>> to a #ifdef section above with a full implementation and a stub
>>> implementation.
>> I'm going to repost patches without #ifdefs for readability.
>> Then we could discuss if we should introduce #ifdefs and how.
> 
> If you could provide a summary of the size increase from your latest post 
> (i.e., size of the kernel before and after) that might lay rest to some 
> theoretical complaints.

These patches add:

* at data structures level:

- one cputime64_t (8 bytes) per cpu
- two cputime_t (2 * 4 bytes) per signal_struct and one cputime_t per task_struct

* at processing level:

- display and convert one more value in show_stat() (fs/proc/proc_misc.c)
- display and convert two more values in do_task_stat() (fs/proc/array.c)
- manage (copy and add) one more value in __exit_signal() (kernel/exit.c)
- initialize three more fields in copy_signal (kernel/fork.c)
- add a bit test in account_system_time() and if the bit is set call a new
function (46 bytes on x86_64).

Size of stripped kernel before (on x86_64):
vmlinux 7305064 bytes (not stripped 52643888 bytes)
Size of stripped kernel after:
vmlinux 7305064 bytes (not stripped 52677180 bytes)

means 0 bytes added ????

Laurent
-- 
------------- Laurent.Vivier@bull.net  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]