linux-kernel - Re: [PATCH v4 5/5] sched: add two functions for att(det)aching a task to(from) a cfs

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: <20150901150343.GT19282@twins.programming.kicks-ass.net>
Date:	Tue, 1 Sep 2015 17:03:43 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Byungchul Park <byungchul.park@....com>
Cc:	mingo@...nel.org, linux-kernel@...r.kernel.org, yuyang.du@...el.com
Subject: Re: [PATCH v4 5/5] sched: add two functions for att(det)aching a
 task to(from) a cfs_rq

On Tue, Sep 01, 2015 at 09:28:49AM +0900, Byungchul Park wrote:

> check the condition "!(flags & DEQUEUE_SLEEP)" for doing normalizing in
> dequeue_entity(). i think you have to keep my original comment, or
> modify your comment to something like below.
> 
> before - If it's !queued, sleeping tasks have a normalized vruntime,
> after - If it's !queued, sleeping tasks have a non-normalize vruntime,
> 
> but.. i think it would be better that you keep my original comment..

The comment we can talk about later, but I think the condition:

> > -	if (p->state == TASK_RUNNING)
> > +	if (!p->se.on_rq)

is important now. Both are broken in different ways.

	p->state == TASK_RUNNING

is broken in this scenario:

	CPU0					CPU1

	set_current_state(TASK_UNINTERRUPTIBLE);

						sched_move_task()
						  task_move_group_fair()
						    vruntime_normalized() == true
	if (!cond)
		schedule();
	__set_current_state(TASK_RUNNING);


Now the proposed replacement:

	!p->se.on_rq

is equally broken, because (as you point out) clearing it isn't
conditional on DEQUEUE_SLEEP.


And the problem with tracking the vruntime state is that while it helps
detach_task_cfs_rq(), attach_task_cfs_rq() is still left wondering what
it should return to.

So we do indeed need something to determine, based on the current state,
if vruntime should be normalized.

/me ponders moar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/