Messages in this thread | | | Date | Fri, 4 Nov 2022 12:09:12 +0800 | From | Chen Yu <> | Subject | Re: [RFC PATCH v2 1/2] sched/fair: Introduce short duration task check |
| |
Hi Peter, thanks for your review, On 2022-11-03 at 13:44:12 +0100, Peter Zijlstra wrote: > On Sun, Oct 23, 2022 at 11:32:31PM +0800, Chen Yu wrote: > > > + if (sched_feat(SIS_SHORT) && !prev->on_rq) { > > + /* > > + * sum_exec_runtime has been updated in update_curr() > > + * because we reach here via dequeue. > > + */ > > + this_dur_avg = se->sum_exec_runtime - se->prev_sum_runtime_vol; > > + /* > > + * Record the accumulated runtime when task voluntarily > > + * switches out. End of old duration period, a new period > > + * starts. > > + */ > > + se->prev_sum_runtime_vol = se->sum_exec_runtime; > > + > > + last_dur_avg = se->dur_avg; > > + delta = this_dur_avg - last_dur_avg; > > + /* consider large change to avoid frequent update */ > > + if (abs(delta) >= sysctl_sched_min_granularity) { > > + /* > > + * If it is the first time the task starts to > > + * record dur_avg, discard the initial value 0. > > + * Otherwise, calculate the EWMA. > > + */ > > + if (unlikely(!this_dur_avg)) > > + se->dur_avg = this_dur_avg; > > + else > > + update_avg(&se->dur_avg, this_dur_avg); > > + } > > + } > > This seems excessively convoluted; what's wrong with something like: > > if (sched_feat(SIS_SHORT) && !prev->on_rq) { > u64 this_dur = se->sum_exec_runtime - se->prev_sum_exec_runtime_vol; > se->prev_sum_exec_runtime_vol = se->sum_exec_runtime; > > update_avg(&se->dur_avg, this_dur); > } > > All that other stuff just makes it unreadable and probably slower. OK, I'll try your suggestion in next version.
thanks, Chenyu
| |