lkml.org 
[lkml]   [2022]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v10 2/7] sched/fair: Decay task PELT values during wakeup migration
On 07/06/2022 14:32, Vincent Donnefort wrote:
> From: Vincent Donnefort <vincent.donnefort@arm.com>

[...]

> To that end, we need sched_clock_cpu() but it is a costly function. Limit
> the usage to the case where the source CPU is idle as we know this is when
> the clock is having the biggest risk of being outdated. In this such case,

s/In this such case/in such a case

> let's call it cfs_idle_lag the delta time between the rq_clock_pelt value
> at rq idle and cfs_rq idle.

s/it cfs_idle_lag the delta time between the rq_clock_pelt value at rq
idle and cfs_rq idle./the delta time between the rq_clock_pelt value
> at rq idle and cfs_rq idle cfs_idle_lag?

And rq_idle_lag the delta between "now" and
> the rq_clock_pelt at rq idle.
>

--->

> The estimated PELT clock is then:
>
> last_update_time (the cfs_rq's last_update_time)
> + cfs_idle_lag (delta between cfs_rq's update and rq's update> + rq_idle_lag (delta between rq's update and now)
>
> last_update_time = cfs_rq_clock_pelt()
> = rq_clock_pelt() - cfs->throttled_clock_pelt_time
>
> cfs_idle_lag = rq_clock_pelt()@rq_idle -
> rq_clock_pelt()@cfs_rq_idle
>
> rq_idle_lag = sched_clock_cpu() - rq_clock()@rq_idle
>
> The rq_clock_pelt() from last_update_time being the same as
> rq_clock_pelt()@cfs_rq_idle, we can write:
>
> estimation = rq_clock_pelt()@rq_idle - cfs->throttled_clock_pelt_time +
> sched_clock_cpu() - rq_clock()@rq_idle
>
> The clocks being not accessible without the rq lock taken, some timestamps
> are created:
>
> rq_clock_pelt()@rq_idle is rq->clock_pelt_idle
> rq_clock()@rq_idle is rq->enter_idle
> cfs->throttled_clock_pelt_time is cfs_rq->throttled_pelt_idle
>

<--- ^^^

This whole block seems to be the same information as the comment block
in migrate_se_pelt_lag(). But you haven't updated it in v10. Maybe you
can get rid of this here and point to the comment block in
migrate_se_pelt_lag() from here instead to guarantee consistency?
Otherwise they should be in sync.

[...]

> + /*
> + * Estimated "now" is: last_update_time + cfs_idle_lag + rq_idle_lag, where:
> + *
> + * last_update_time (the cfs_rq's last_update_time)
> + * = cfs_rq_clock_pelt()@cfs_rq_idle
> + * = rq_clock_pelt()@cfs_rq_idle
> + * - cfs->throttled_clock_pelt_time@cfs_rq_idle
> + *
> + * cfs_idle_lag (delta between cfs_rq's update and rq's update)

Isn't this: (delta between rq's update and cfs_rq's update) ?

> + * = rq_clock_pelt()@rq_idle - rq_clock_pelt()@cfs_rq_idle
> + *
> + * rq_idle_lag (delta between rq's update and now)

Isn't this: (delta between now and rq's update) ?

> + * = sched_clock_cpu() - rq_clock()@rq_idle
> + *
> + * We can then write:
> + *
> + * now = rq_clock_pelt()@rq_idle - cfs->throttled_clock_pelt_time +
> + * sched_clock_cpu() - rq_clock()@rq_idle
> + * Where:
> + * rq_clock_pelt()@rq_idle is rq->clock_pelt_idle
> + * rq_clock()@rq_idle is rq->clock_idle
is rq->clock_pelt_idle
is rq->clock_idle

> + * cfs->throttled_clock_pelt_time@cfs_rq_idle is
> + * cfs_rq->throttled_pelt_idle
is cfs_rq->throttled_pelt_idle

Maybe align the `is foo` for readability?

[...]

\
 
 \ /
  Last update: 2022-06-08 12:12    [W:0.155 / U:0.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site