From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Vincent Donnefort <vincent.donnefort@arm.com>
Cc: peterz@infradead.org, mingo@redhat.com,
vincent.guittot@linaro.org, linux-kernel@vger.kernel.org,
valentin.schneider@arm.com, morten.rasmussen@arm.com,
chris.redpath@arm.com, qperret@google.com, lukasz.luba@arm.com
Subject: Re: [PATCH 2/4] sched/fair: Decay task PELT values during migration
Date: Tue, 21 Dec 2021 13:46:01 +0100 [thread overview]
Message-ID: <2551684c-c987-b143-ba69-4fb0c55f61c7@arm.com> (raw)
In-Reply-To: <20211220155735.GA51378@ubiquitous>
On 20.12.21 17:09, Vincent Donnefort wrote:
> On Mon, Dec 20, 2021 at 12:26:23PM +0100, Dietmar Eggemann wrote:
>> On 09.12.21 17:11, Vincent Donnefort wrote:
[...]
>> Why do you use `avg.last_update_time` (lut) of the root cfs_rq here?
>>
>> p's lut was just synced to cfs_rq_of(se)'s lut in
>>
>> migrate_task_rq_fair() (1) -> remove_entity_load_avg() ->
>> sync_entity_load_avg(se) (2)
>
> Huum, indeed, the estimation is an offset on top of the se's last_update_time,
> which I suppose could be different from the rq's cfs_rq.
>
> I'll add a sched_entity argument for this function, to use either cfs_rq_of(se)
> or se last_update_time
OK, or an `u64 now or lut`.
[...]
>>> } else {
>>> + remove_entity_load_avg(se);
>>> +
>>> /*
>>> - * We are supposed to update the task to "current" time, then
>>> - * its up to date and ready to go to new CPU/cfs_rq. But we
>>> - * have difficulty in getting what current time is, so simply
>>> - * throw away the out-of-date time. This will result in the
>>> - * wakee task is less decayed, but giving the wakee more load
>>> - * sounds not bad.
>>> + * Here, the task's PELT values have been updated according to
>>> + * the current rq's clock. But if that clock hasn't been
>>> + * updated in a while, a substantial idle time will be missed,
>>> + * leading to an inflation after wake-up on the new rq.
>>> + *
>>> + * Estimate the PELT clock lag, and update sched_avg to ensure
>>> + * PELT continuity after migration.
>>> */
>>> - remove_entity_load_avg(&p->se);
>>> + __update_load_avg_blocked_se(rq_clock_pelt_estimator(rq), se);
>>
>> We do __update_load_avg_blocked_se() now twice for p, 1. in (2) and then
>> in (1) again.
>
> the first __update_load_avg_blocked_se() ensures the se is aligned with the
> cfs_rq's clock and then, update the "removed" struct accordingly. We couldn't
> use the estimator there, it would break that structure.
You're right. I missed this bit.
Related to this: Looks like on CAS/EAS we don't rely on
remove_entity_load_avg()->sync_entity_load_avg(se) since it is already
called during select_task_rq().
next prev parent reply other threads:[~2021-12-21 12:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 16:11 [PATCH 0/4] feec() energy margin removal Vincent Donnefort
2021-12-09 16:11 ` [PATCH 1/4] sched/fair: Provide u64 read for 32-bits arch helper Vincent Donnefort
2021-12-09 16:11 ` [PATCH 2/4] sched/fair: Decay task PELT values during migration Vincent Donnefort
2021-12-20 11:26 ` Dietmar Eggemann
2021-12-20 16:09 ` Vincent Donnefort
2021-12-21 12:46 ` Dietmar Eggemann [this message]
2021-12-09 16:11 ` [PATCH 3/4] sched/fair: Remove task_util from effective utilization in feec() Vincent Donnefort
2022-01-04 17:27 ` Dietmar Eggemann
2022-01-12 10:20 ` Vincent Donnefort
2021-12-09 16:11 ` [PATCH 4/4] sched/fair: Remove the energy margin " Vincent Donnefort
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2551684c-c987-b143-ba69-4fb0c55f61c7@arm.com \
--to=dietmar.eggemann@arm.com \
--cc=chris.redpath@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lukasz.luba@arm.com \
--cc=mingo@redhat.com \
--cc=morten.rasmussen@arm.com \
--cc=peterz@infradead.org \
--cc=qperret@google.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.donnefort@arm.com \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).