lkml.org 
[lkml]   [2015]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/1] cputime: Make the reported utime+stime correspond to the actual runtime.
I don't think that is good enough. I believe the reason the
max()-stuff was initially put there to make sure the returned stime
and utime components are increasing monotonically. The scaling code
can cause either or to decrease from one call to the other even i
rtime increases.

The purpose of my patch is to also make sure that the sum of utime and
stime is rtime.

I lost the last part of the patch in my previous email:


- cputime_advance(&prev->stime, stime);
- cputime_advance(&prev->utime, utime);
+ if (stime < prev->stime) {
+ stime = prev->stime;
+ utime = rtime - stime;
+ } else if (utime < prev->utime) {
+ utime = prev->utime;
+ stime = rtime - utime;
+ }
-out:
+ if (prev->stime + prev->utime < rtime) {
+ prev->stime = stime;
+ prev->utime = utime;
+ }
*ut = prev->utime;
*st = prev->stime;


/Fredrik

On Mon, Jun 29, 2015 at 8:54 PM, Jason Low <jason.low2@hp.com> wrote:
> On Mon, 2015-06-29 at 17:28 +0200, Fredrik Markström wrote:
>> Hello Peter, the locking part looks good, I don't have a strong
>> opinion on per task/signal lock vs global lock.
>>
>> But with the patch we still update prev->utime and prev->stime
>> independently, which was the original problem. But maybe the locking
>> and monoticity/sum issue should be addressed by two separate patches
>> since they are separate bugs ?
>>
>> The part I'm referring to is the change below from my original patch
>> (maybe without the WARN_ON:s ?):
>>
>> …
>> - cputime_advance(&prev->stime, stime);
>> - cputime_advance(&prev->utime, utime);
>> + if (stime < prev->stime) {
>> + stime = prev->stime;
>> + utime = rtime - stime;
>> + } else if (utime < prev->utime) {
>> + utime = prev->utime;
>> + stime = rtime - utime;
>> + }
>> + WARN_ON(stime < prev->stime);
>> + WARN_ON(utime < prev->utime);
>> + WARN_ON(stime + utime != rtime);
>
> How about substituting:
>
> prev->stime = max(prev->stime, stime);
> prev->utime = max(prev->utime, utime);
>
> with
>
> if (stime > prev->stime || utime > prev->utime) {
> prev->stime = stime;
> prev->utime = utime;
> }
>
> in Peter's patch?
>



--
/Fredrik


\
 
 \ /
  Last update: 2015-06-29 21:21    [W:0.099 / U:1.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site