Re: [PATCH 1/1] cputime: Make the reported utime+stime correspond to the actual runtime.

[Fredrik MarkstrÃm]

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@xxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/