nice value is ignored on cpu time accounting of a guest?

nice value is ignored on cpu time accounting of a guest?

[Ryota Ozaki]

Hi,

I have a question on cputime accounting of a guest. CPU time of a
guest is always
accounted as 'user' time of cpustat even if nice value of the guest is
higher than 0.
Is there a reason to do so? I think the cpu time of the guest should
be accounted
into 'nice' as same as a normal process. Am I wrong?

  ozaki-r

Re: nice value is ignored on cpu time accounting of a guest?

[Avi Kivity]

On 10/19/2009 06:46 PM, Ryota Ozaki wrote:
> Hi,
>
> I have a question on cputime accounting of a guest. CPU time of a
> guest is always
> accounted as 'user' time of cpustat even if nice value of the guest is
> higher than 0.
> Is there a reason to do so? I think the cpu time of the guest should
> be accounted
> into 'nice' as same as a normal process. Am I wrong?
>    

Hm, guest time is accounted separately, and added to user time in /proc 
(so tools that don't know about guest time can read it as user time).

Looks like we need to add a separate guest_nice, or get rid of guest 
time altogether.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: nice value is ignored on cpu time accounting of a guest?

[Ryota Ozaki]

On Tue, Oct 20, 2009 at 2:21 PM, Avi Kivity <avi@redhat.com> wrote:
> On 10/19/2009 06:46 PM, Ryota Ozaki wrote:
>>
>> Hi,
>>
>> I have a question on cputime accounting of a guest. CPU time of a
>> guest is always
>> accounted as 'user' time of cpustat even if nice value of the guest is
>> higher than 0.
>> Is there a reason to do so? I think the cpu time of the guest should
>> be accounted
>> into 'nice' as same as a normal process. Am I wrong?
>>
>
> Hm, guest time is accounted separately, and added to user time in /proc (so
> tools that don't know about guest time can read it as user time).

Yes, but I think always added to user time without regard to nice
value is a problem.
I want to fix it because user time is an account for processes that
have nice == 0.

>
> Looks like we need to add a separate guest_nice, or get rid of guest time
> altogether.

Hmm, guest time is already exposed via /proc/stat so adding guest_nice is better
if fix here? I don't know anyone utilize 'guest' value though.

  ozaki-r

>
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
>

Re: nice value is ignored on cpu time accounting of a guest?

[Avi Kivity]

On 10/20/2009 04:06 PM, Ryota Ozaki wrote:
>> Looks like we need to add a separate guest_nice, or get rid of guest time
>> altogether.
>>      
> Hmm, guest time is already exposed via /proc/stat so adding guest_nice is better
> if fix here? I don't know anyone utilize 'guest' value though.
>    

No one uses guest time to my knowledge.  However, we can't be sure, so 
it's better to add guest_nice.

Note you need to add guest_nice to user_nice, so old tools see it as 
nice time (same as guest_time now).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: nice value is ignored on cpu time accounting of a guest?

[Ryota Ozaki]

On Tue, Oct 20, 2009 at 4:17 PM, Avi Kivity <avi@redhat.com> wrote:
> On 10/20/2009 04:06 PM, Ryota Ozaki wrote:
>>>
>>> Looks like we need to add a separate guest_nice, or get rid of guest time
>>> altogether.
>>>
>>
>> Hmm, guest time is already exposed via /proc/stat so adding guest_nice is
>> better
>> if fix here? I don't know anyone utilize 'guest' value though.
>>
>
> No one uses guest time to my knowledge.  However, we can't be sure, so it's
> better to add guest_nice.
>
> Note you need to add guest_nice to user_nice, so old tools see it as nice
> time (same as guest_time now).

Well, like this?

        /* Add user time to cpustat. */
        tmp = cputime_to_cputime64(cputime);
        if (TASK_NICE(p) > 0) {
                cpustat->nice = cputime64_add(cpustat->nice, tmp);
                cpustat->guest_nice = cputime64_add(cpustat->guest_nice, tmp);
        } else {
                cpustat->user = cputime64_add(cpustat->user, tmp);
                cpustat->guest = cputime64_add(cpustat->guest, tmp);
        }

  ozaki-r

Re: nice value is ignored on cpu time accounting of a guest?

[Avi Kivity]

On 10/20/2009 04:27 PM, Ryota Ozaki wrote:
> On Tue, Oct 20, 2009 at 4:17 PM, Avi Kivity<avi@redhat.com>  wrote:
>    
>> On 10/20/2009 04:06 PM, Ryota Ozaki wrote:
>>      
>>>> Looks like we need to add a separate guest_nice, or get rid of guest time
>>>> altogether.
>>>>
>>>>          
>>> Hmm, guest time is already exposed via /proc/stat so adding guest_nice is
>>> better
>>> if fix here? I don't know anyone utilize 'guest' value though.
>>>
>>>        
>> No one uses guest time to my knowledge.  However, we can't be sure, so it's
>> better to add guest_nice.
>>
>> Note you need to add guest_nice to user_nice, so old tools see it as nice
>> time (same as guest_time now).
>>      
> Well, like this?
>
>          /* Add user time to cpustat. */
>          tmp = cputime_to_cputime64(cputime);
>          if (TASK_NICE(p)>  0) {
>                  cpustat->nice = cputime64_add(cpustat->nice, tmp);
>                  cpustat->guest_nice = cputime64_add(cpustat->guest_nice, tmp);
>          } else {
>                  cpustat->user = cputime64_add(cpustat->user, tmp);
>                  cpustat->guest = cputime64_add(cpustat->guest, tmp);
>          }
>    

In account_guest_time()?  Yes.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: nice value is ignored on cpu time accounting of a guest?

[Ryota Ozaki]

On Tue, Oct 20, 2009 at 4:34 PM, Avi Kivity <avi@redhat.com> wrote:
> On 10/20/2009 04:27 PM, Ryota Ozaki wrote:

>> Well, like this?
>>
>>         /* Add user time to cpustat. */
>>         tmp = cputime_to_cputime64(cputime);
>>         if (TASK_NICE(p)>  0) {
>>                 cpustat->nice = cputime64_add(cpustat->nice, tmp);
>>                 cpustat->guest_nice = cputime64_add(cpustat->guest_nice,
>> tmp);
>>         } else {
>>                 cpustat->user = cputime64_add(cpustat->user, tmp);
>>                 cpustat->guest = cputime64_add(cpustat->guest, tmp);
>>         }
>>
>
> In account_guest_time()?  Yes.

Yes.

OK, I'll send a patch later. Thanks!

  ozaki-r