lkml.org 
[lkml]   [2017]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH -v2 15/18] sched/fair: Align PELT windows between cfs_rq and its se
From
Date
On 06/10/17 14:02, Peter Zijlstra wrote:
> On Wed, Oct 04, 2017 at 08:27:01PM +0100, Dietmar Eggemann wrote:
>> On 01/09/17 14:21, Peter Zijlstra wrote:
>>> The PELT _sum values are a saw-tooth function, dropping on the decay
>>> edge and then growing back up again during the window.
>>>
>>> When these window-edges are not aligned between cfs_rq and se, we can
>>> have the situation where, for example, on dequeue, the se decays
>>> first.
>>>
>>> Its _sum values will be small(er), while the cfs_rq _sum values will
>>> still be on their way up. Because of this, the subtraction:
>>> cfs_rq->avg._sum -= se->avg._sum will result in a positive value. This
>>> will then, once the cfs_rq reaches an edge, translate into its _avg
>>> value jumping up.
>>>
>>> This is especially visible with the runnable_load bits, since they get
>>> added/subtracted a lot.
>>>
>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>> ---
>>> kernel/sched/fair.c | 45 +++++++++++++++++++++++++++++++--------------
>>> 1 file changed, 31 insertions(+), 14 deletions(-)
>>
>> [...]
>>
>>> @@ -3644,7 +3634,34 @@ update_cfs_rq_load_avg(u64 now, struct c
>>> */
>>> static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
>>> {
>>> + u32 divider = LOAD_AVG_MAX - 1024 + cfs_rq->avg.period_contrib;
>>> +
>>> + /*
>>> + * When we attach the @se to the @cfs_rq, we must align the decay
>>> + * window because without that, really weird and wonderful things can
>>> + * happen.
>>> + *
>>> + * XXX illustrate
>>> + */
>>> se->avg.last_update_time = cfs_rq->avg.last_update_time;
>>> + se->avg.period_contrib = cfs_rq->avg.period_contrib;
>>> +
>>> + /*
>>> + * Hell(o) Nasty stuff.. we need to recompute _sum based on the new
>>> + * period_contrib. This isn't strictly correct, but since we're
>>> + * entirely outside of the PELT hierarchy, nobody cares if we truncate
>>> + * _sum a little.
>>> + */
>>> + se->avg.util_sum = se->avg.util_avg * divider;
>>> +
>>> + se->avg.load_sum = divider;
>>> + if (se_weight(se)) {
>>> + se->avg.load_sum =
>>> + div_u64(se->avg.load_avg * se->avg.load_sum, se_weight(se));
>>> + }
>>
>> Can scale_load_down(se->load.weight) ever become 0 here?
>
> Yeah, don't see why not.

Tasks should be safe since for them scale_load_down(se->load.weight)
should be >= 15 when they get attached.

task groups get attached via attach_entity_cfs_rq()

# mkdir /sys/fs/cgroup/foo

[ 63.885333] [<ffff00000811a558>] attach_entity_load_avg+0x138/0x558
[ 63.885345] [<ffff0000081208e0>] attach_entity_cfs_rq+0x298/0x998
[ 63.885357] [<ffff00000812d660>] online_fair_sched_group+0x70/0xb0
[ 63.885369] [<ffff0000081182d4>] sched_online_group+0x94/0xb0
[ 63.885381] [<ffff000008118318>] cpu_cgroup_css_online+0x28/0x38
[ 63.885393] [<ffff0000081a5e80>] online_css+0x38/0xd0
[ 63.885406] [<ffff0000081acae0>] cgroup_apply_control_enable+0x260
[ 63.885418] [<ffff0000081afc54>] cgroup_mkdir+0x314/0x4e8

mkdir-2501 [004] 63.689455: bprint:
attach_entity_load_avg: cpu=1 se=0xffff800072fffc00 load.weight=1048576

If I apply the smallest shares possible to that task group, it is
already attched.

# echo 2 > /sys/fs/cgroup/foo/cpu.shares

create_cpu_cgro-2500 [003] 64.591878: bprint:
update_cfs_group: cpu=1 se=0xffff800072fffc00 tg->shares=2048 shares=0
load=0 tg_weight=0
create_cpu_cgro-2500 [003] 64.591904: bprint:
reweight_entity: cpu=1 se=0xffff800072fffc00 se->load.weight=2

I can't see right now how a task group can get attached with
se->load.weight < 1024 (32 bit)/1048576 (64 bit)? Do I miss something?

\
 
 \ /
  Last update: 2017-10-09 14:16    [W:0.247 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site