linux-kernel - Re: [PATCH 7/7 v3] sched: fix wrong utilization accounting when switching to fair class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160915131807.GS5008@twins.programming.kicks-ass.net>
Date:   Thu, 15 Sep 2016 15:18:07 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Vincent Guittot <vincent.guittot@...aro.org>
Cc:     mingo@...nel.org, linux-kernel@...r.kernel.org,
        yuyang.du@...el.com, Morten.Rasmussen@....com,
        linaro-kernel@...ts.linaro.org, dietmar.eggemann@....com,
        pjt@...gle.com, bsegall@...gle.com
Subject: Re: [PATCH 7/7 v3] sched: fix wrong utilization accounting when
 switching to fair class

On Mon, Sep 12, 2016 at 09:47:52AM +0200, Vincent Guittot wrote:
> When a task switches to fair scheduling class, the period between now and
> the last update of its utilization is accounted as running time whatever
> happened during this period. This wrong accounting applies to the task
> and also to the task group branch.
> 
> When changing the property of a running task like its list of allowed CPUs
> or its scheduling class, we follow the sequence:
> -dequeue task
> -put task
> -change the property
> -set task as current task
> -enqueue task
> 
> The end of the sequence doesn't follow the normal sequence which is :
> -enqueue a task
> -then set the task as current task.
> 
> This wrong ordering is the root cause of wrong utilization accounting.
> Update the sequence to follow the right one:
> -dequeue task
> -put task
> -change the property
> -enqueue task
> -set task as current task

But enqueue_entity depends on cfs_rq->curr, which is set by
set_curr_task_fair().

Also, the normalize comment in dequeue_entity() worries me, 'someone'
didn't update that when he moved update_min_vruntime() around.