Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1678007

Re: [kernel-sched-cputime] question about probable bug in cputime_adjust()

Show all headers | View raw

Quoting Frans Klaver <fransklaver@gmail.com>:

> On 29 June 2017 01:57:19 CEST, "Gustavo A. R. Silva"  
> <garsilva@embeddedor.com> wrote:
>>>>> --- a/kernel/sched/cputime.c
>>>>> +++ b/kernel/sched/cputime.c
>>>>> @@ -637,9 +637,10 @@ static void cputime_adjust(struct task_cputime
>> *curr,
>>>>>          *            = (rtime_i+1 - rtime_i) + utime_i
>>>>>          *            >= utime_i
>>>>>          */
>>>>> -       if (stime < prev->stime)
>>>>> +       if (stime < prev->stime) {
>>>>>                 stime = prev->stime;
>>>>> -       utime = rtime - stime;
>>>>> +               utime = rtime - stime;
>>>>> +       }
>>>>>
>>>>>
>>>>> If you confirm this, I will send a patch in a full and proper form.
>>>>>
>>>>> I'd really appreciate your comments.
>>>>
>>>> If you do that, how would you meet the guarantee made in line 583?
>>>>
>>
>> You are right, I see now.
>>
>> Then in this case the following patch would be the way to go:
>>
>> --- a/kernel/sched/cputime.c
>> +++ b/kernel/sched/cputime.c
>> @@ -615,10 +615,8 @@ static void cputime_adjust(struct task_cputime
>> *curr,
>>       * userspace. Once a task gets some ticks, the monotonicy code at
>>          * 'update' will ensure things converge to the observed ratio.
>>          */
>> -       if (stime == 0) {
>> -               utime = rtime;
>> +       if (stime == 0)
>>                 goto update;
>> -       }
>>
>>         if (utime == 0) {
>>                 stime = rtime;
>>
>>
>> but I think this one is even better:
>>
>>
>> --- a/kernel/sched/cputime.c
>> +++ b/kernel/sched/cputime.c
>> @@ -615,19 +615,11 @@ static void cputime_adjust(struct task_cputime
>> *curr,
>>       * userspace. Once a task gets some ticks, the monotonicy code at
>>          * 'update' will ensure things converge to the observed ratio.
>>          */
>> -       if (stime == 0) {
>> -               utime = rtime;
>> -               goto update;
>> -       }
>> -
>> -       if (utime == 0) {
>> +       if (stime != 0 && utime == 0)
>>                 stime = rtime;
>> -               goto update;
>> -       }
>> -
>> -       stime = scale_stime(stime, rtime, stime + utime);
>> +       else
>> +               stime = scale_stime(stime, rtime, stime + utime);
>
> I don't think it is better. The stime == 0 case is gone now. So  
> scale_time() will be called in that case. This whole if/else block  
> should only be executed if stime != 0.

Oh yeah! something like:

if (stime != 0) {
         if (stime != 0 && utime == 0)
                 stime = rtime;
         else
                 stime = scale_stime(stime, rtime, stime + utime);
}

I'll be right back with the final patch.

Thanks for your time, Frans.
Much appreciated :)
--
Gustavo A. R. Silva

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[kernel-sched-cputime] question about probable bug in  cputime_adjust() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-28 01:10 +0200
  Re: [kernel-sched-cputime] question about probable bug in cputime_adjust() Frans Klaver <fransklaver@gmail.com> - 2017-06-28 07:40 +0200
    Re: [kernel-sched-cputime] question about probable bug in cputime_adjust() Frans Klaver <fransklaver@gmail.com> - 2017-06-28 08:10 +0200
      Re: [kernel-sched-cputime] question about probable bug in  cputime_adjust() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-29 02:30 +0200
        Re: [kernel-sched-cputime] question about probable bug in cputime_adjust() Frans Klaver <fransklaver@gmail.com> - 2017-06-29 07:00 +0200
          Re: [kernel-sched-cputime] question about probable bug in  cputime_adjust() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-29 20:00 +0200
            [PATCH] sched/cputime: code refactoring in cputime_adjust() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-29 20:50 +0200
              [tip:sched/core] sched/cputime: Refactor the cputime_adjust() code "tip-bot for Gustavo A. R. Silva" <tipbot@zytor.com> - 2017-06-30 15:20 +0200
                Re: [tip:sched/core] sched/cputime: Refactor the cputime_adjust()  code Rik van Riel <riel@redhat.com> - 2017-06-30 16:10 +0200
                Re: [tip:sched/core] sched/cputime: Refactor the cputime_adjust() code Frans Klaver <fransklaver@gmail.com> - 2017-06-30 16:50 +0200
                Re: [tip:sched/core] sched/cputime: Refactor the cputime_adjust()  code Frederic Weisbecker <fweisbec@gmail.com> - 2017-06-30 17:50 +0200
                Re: [tip:sched/core] sched/cputime: Refactor the cputime_adjust()  code Stanislaw Gruszka <sgruszka@redhat.com> - 2017-06-30 18:20 +0200

csiph-web