linux-kernel - Re: [PATCH 2/2] sched/fair: Skip detach and attach load avgs for new group task

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]

Date:	Fri, 27 May 2016 15:37:11 +0200
From:	Vincent Guittot <vincent.guittot@...aro.org>
To:	Yuyang Du <yuyang.du@...el.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Benjamin Segall <bsegall@...gle.com>,
	Paul Turner <pjt@...gle.com>,
	Morten Rasmussen <morten.rasmussen@....com>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	byungchul.park@....com
Subject: Re: [PATCH 2/2] sched/fair: Skip detach and attach load avgs for new
 group task

On 26 May 2016 at 21:44, Yuyang Du <yuyang.du@...el.com> wrote:
> Hi Vincent,
>
> On Thu, May 26, 2016 at 01:50:56PM +0200, Vincent Guittot wrote:
>> On 26 May 2016 at 03:14, Yuyang Du <yuyang.du@...el.com> wrote:
>> > Vincent reported that the first task to a new task group's cfs_rq will
>> > be attached in attach_task_cfs_rq() and once more when it is enqueued
>> > (see https://lkml.org/lkml/2016/5/25/388).
>> >
>> > Actually, it is worse, attach_task_cfs_rq() is called for new task even
>> > way before init_entity_runnable_average().
>> >
>> > Solve this by avoiding attach as well as detach new task's sched avgs
>> > in task_move_group_fair(). To do it, we need to know whether the task
>> > is forked or not, so we pass this info all the way from sched_move_task()
>> > to attach_task_cfs_rq().
>>
>> Not sure that this is the right way to solve this problem because you
>> continue to attach the task twice without detaching it in the mean
>> time:
>> - once during the copy of the process in cpu_cgroup_fork (you skip the
>> attach of load average but the task is still attached to the local
>> cpu)
>
> Sorry, the task's what is still attached, and how? You mean the vruntime
> thingy? But the load/util avgs are not.

yes that's it. The sequence still looks weird IMHO.
the detach is called for a newly forked task that is not fully init
and has not been attached yet
IIUC the fork sequence, we only need to set group at this point so you
can skip completely the detach/attach_task_cfs_rq not only the
detach/attach_entity_load_avg

>
>> In the mean time, sched_entity is initialized and the last_update_time is reset
>
> last_update_time is set to 0 in initialization, and this is the first time
> it is touched, no?
>
>> - one more time when the task is enqueued because the last_update_time
>> has been reset (this time you don't skip the attache of load_avg
>
> This is expected/wanted. We don't skip this because this will be the first-time
> attach.
>
>> Should you better detach the sched_entity with a copy of its parent
>> metrics before initializing it and attaching it to the new cpu ?
>
> Thanks,
> Yuyang