Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-26 Thread Liangyan
On 19/8/27 上午1:38, bseg...@google.com wrote: Valentin Schneider writes: On 23/08/2019 21:00, bseg...@google.com wrote: [...] Could you mention in the message that this a throttled cfs_rq can have account_cfs_rq_runtime called on it because it is throttled before idle_balance, and the

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-26 Thread bsegall
Valentin Schneider writes: > On 23/08/2019 21:00, bseg...@google.com wrote: > [...] >> Could you mention in the message that this a throttled cfs_rq can have >> account_cfs_rq_runtime called on it because it is throttled before >> idle_balance, and the idle_balance calls update_rq_clock to add

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-23 Thread Valentin Schneider
On 23/08/2019 21:00, bseg...@google.com wrote: [...] > Could you mention in the message that this a throttled cfs_rq can have > account_cfs_rq_runtime called on it because it is throttled before > idle_balance, and the idle_balance calls update_rq_clock to add time > that is accounted to the task.

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-23 Thread bsegall
Liangyan writes: > do_sched_cfs_period_timer() will refill cfs_b runtime and call > distribute_cfs_runtime() to unthrottle cfs_rq, sometimes cfs_b->runtime > will allocate all quota to one cfs_rq incorrectly. > This will cause other cfs_rq can't get runtime and will be throttled. > We find that

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-19 Thread Valentin Schneider
On 16/08/2019 18:19, Valentin Schneider wrote: [...] > Yeah it's probably pretty stupid. IIRC throttled cfs_rq means frozen > rq_clock, so any subsequent call to update_curr() on a throttled cfs_rq > should lead to an early bailout anyway due to delta_exec <= 0. > Did some more tracing, seems

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-16 Thread Valentin Schneider
On 16/08/2019 16:39, Liangyan wrote: > Thanks for the feedback. > Add some debug prints and get below log. It seems that pick_next_task_fair > throttle the cfs_rq first, then call put_prev_entity to assign runtime to > this cfs_rq. > [...] > > Regarding the suggested change, i’m not sure

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-16 Thread Valentin Schneider
On 16/08/2019 15:02, Valentin Schneider wrote: > On 16/08/2019 08:08, Liangyan wrote: >> Please check below dmesg log with “WARN_ON(cfs_rq->runtime_remaining > 0)”. >> If apply my patch, the warning is gone. Append the reproducing case in the >> end. >> > > [...] > > Huh, thanks for the log &

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-16 Thread Valentin Schneider
On 16/08/2019 08:08, Liangyan wrote: > Please check below dmesg log with “WARN_ON(cfs_rq->runtime_remaining > 0)”. > If apply my patch, the warning is gone. Append the reproducing case in the > end. > [...] Huh, thanks for the log & the reproducer. I'm still struggling to understand how we

Re: [PATCH] sched/fair: don't assign runtime for throttled cfs_rq

2019-08-15 Thread Valentin Schneider
On 14/08/2019 19:00, Liangyan wrote: > do_sched_cfs_period_timer() will refill cfs_b runtime and call > distribute_cfs_runtime() to unthrottle cfs_rq, sometimes cfs_b->runtime > will allocate all quota to one cfs_rq incorrectly. > This will cause other cfs_rq can't get runtime and will be