Messages in this thread | | | Date | Thu, 29 Jun 2017 12:58:04 -0500 | From | "Gustavo A. R. Silva" <> | Subject | Re: [kernel-sched-cputime] question about probable bug in cputime_adjust() |
| |
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
| |