Messages in this thread | | | Date | Wed, 30 Nov 2022 19:14:51 +0100 | Subject | Re: [RFC PATCH 0/1] sched/pelt: Change PELT halflife at runtime | From | Dietmar Eggemann <> |
| |
On 10/11/2022 18:51, Peter Zijlstra wrote: > On Thu, Nov 10, 2022 at 03:59:01PM +0100, Dietmar Eggemann wrote: >> On 10/11/2022 14:05, Peter Zijlstra wrote: >>> On Thu, Nov 10, 2022 at 12:16:26PM +0100, Dietmar Eggemann wrote: >>>> On 07/11/2022 14:41, Peter Zijlstra wrote: >>>>> On Thu, Sep 29, 2022 at 03:41:47PM +0100, Kajetan Puchalski wrote: >>>> >>>> [...] >>>> >>>>> @@ -2956,13 +2958,26 @@ static inline unsigned long cpu_util_dl(struct rq *rq) >>>>> */ >>>>> static inline unsigned long cpu_util_cfs(int cpu) >>>>> { >>>>> + struct rq *rq = cpu_rq(cpu); >>>>> struct cfs_rq *cfs_rq; >>>>> unsigned long util; >>>>> >>>>> - cfs_rq = &cpu_rq(cpu)->cfs; >>>>> + cfs_rq = &rq->cfs; >>>>> util = READ_ONCE(cfs_rq->avg.util_avg); >>>>> >>>>> if (sched_feat(UTIL_EST)) { >>>>> + if (sched_feat(UTIL_EST_FASTER)) { >>>>> + struct task_struct *curr; >>>>> + >>>>> + rcu_read_lock(); >>>>> + curr = rcu_dereference(rq->curr); >>>>> + if (likely(curr->sched_class == &fair_sched_class)) { >>>>> + u64 runtime = curr->se.sum_exec_runtime - curr->se.exec_start; >>>> >>>> Don't we and up with gigantic runtime numbers here? >>>> >>>> oot@juno:~# cat /proc/1676/task/1676/schedstat >>>> 36946300 1150620 11 >>>> root@juno:~# cat /proc/1676/task/1676/sched >>>> rt-app (1676, #threads: 2) >>>> ------------------------------------------------------------------- >>>> se.exec_start : 77766.964240 <- ! >>>> se.vruntime : 563.587883 >>>> e.sum_exec_runtime : 36.946300 <- ! >>>> se.nr_migrations : 0 >>>> ... >>>> >>>> I expect cpu_util_cfs() to be ~1024 almost all the time now. >>> >>> Duh, obviously I meant to measure the runtime of the current activation >>> and messed up. >>> >>> We don't appear to have the right information to compute this atm :/ >> >> This would be: >> >> u64 now = rq_clock_task(rq); >> u64 runtime = now - curr->se.exec_start; >> >> but we don't hold the rq lock so we can't get `now`? > > Not quite the same; that's the time since we got on-cpu last, but that's > not the same as the runtime of this activation (it is when you discount > preemption).
----|----|----|----|----|----|---> a s1 p1 s2 p2 d
a ... activate_task() -> enqueue_task()
s ... set_next_entity()
p ... put_prev_entity()
d ... deactivate_task() -> dequeue_task()
By `runtime of the activation` you refer to `curr->sum_exec_runtime - time(a)` ? And the latter we don't have?
And `runtime = curr->se.sum_exec_runtime - curr->se.prev_sum_exec_run` is only covering the time since we got onto the cpu, right?
With a missing `runtime >>= 10` (from __update_load_sum()) and using `runtime = curr->se.sum_exec_runtime - curr->se.prev_sum_exec_runtime` for a 1 task-workload (so no preemption) with factor 2 or 4 I get at least close to the original rq->cfs.avg.util_avg and util_est.enqueued signals (cells (5)-(8) in the notebook below).
https://nbviewer.org/github/deggeman/lisa/blob/ipynbs/ipynb/scratchpad/UTIL_EST_FASTER.ipynb?flush_cache=true
----
set_next_entity() update_stats_curr_start() se->exec_start = rq_clock_task()
cfs_rq->curr = se (1)
se->prev_sum_exec_runtime = se->sum_exec_runtime (2)
update_curr()
now = rq_clock_task(rq_of(cfs_rq)) delta_exec = now - curr->exec_start (3) curr->exec_start = now curr->sum_exec_runtime += delta_exec; (4)
put_prev_entity()
cfs_rq->curr = NULL (5)
| |