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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKdL+dSM=_NERJ6U3rky-t=yiTkh3o_GFC-Bwy0dPVmTFBU+iQ@mail.gmail.com>
Date:	Mon, 29 Jun 2015 21:08:34 +0200
From:	Fredrik Markström <fredrik.markstrom@...il.com>
To:	Jason Low <jason.low2@...com>
Cc:	Peter Zijlstra <peterz@...radead.org>, mingo@...hat.com,
	linux-kernel@...r.kernel.org, Rik van Riel <riel@...hat.com>,
	Frédéric Weisbecker <fweisbec@...il.com>
Subject: Re: [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@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/