Messages in this thread | | | From | bsegall@google ... | Subject | Re: [PATCH] sched/fair: don't restart enqueued cfs quota slack timer | Date | Mon, 03 Jun 2019 13:23:47 -0700 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Mon, Jun 03, 2019 at 12:43:09PM +0800, Liangyan wrote: >> From: "liangyan.ply" <liangyan.ply@linux.alibaba.com> >> >> start_cfs_slack_bandwidth() will restart the quota slack timer, >> if it is called frequently, this timer will be restarted continuously >> and may have no chance to expire to unthrottle cfs tasks. >> This will cause that the throttled tasks can't be unthrottled in time >> although they have remaining quota. > > This looks very similar to the patch from Ben here: > > https://lkml.kernel.org/r/xm26blzlyr9d.fsf@bsegall-linux.svl.corp.google.com
Yeah, it should do the same sort of thing, but testing hrtimer_active is racy (we could miss restarting the timer if it's just dropped the cfsb lock but hasn't finished yet), so the extra accounting flag is needed.
> >> >> Signed-off-by: Liangyan <liangyan.peng@linux.alibaba.com> >> --- >> kernel/sched/fair.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index d90a64620072..fdb03c752f97 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -4411,9 +4411,11 @@ static void start_cfs_slack_bandwidth(struct cfs_bandwidth *cfs_b) >> if (runtime_refresh_within(cfs_b, min_left)) >> return; >> >> - hrtimer_start(&cfs_b->slack_timer, >> + if (!hrtimer_active(&cfs_b->slack_timer)) { >> + hrtimer_start(&cfs_b->slack_timer, >> ns_to_ktime(cfs_bandwidth_slack_period), >> HRTIMER_MODE_REL); >> + } >> } >> >> /* we know any runtime found here is valid as update_curr() precedes return */ >> -- >> 2.14.4.44.g2045bb6 >>
| |