From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754368AbbIAPDw (ORCPT ); Tue, 1 Sep 2015 11:03:52 -0400 Received: from casper.infradead.org ([85.118.1.10]:46603 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754255AbbIAPDu (ORCPT ); Tue, 1 Sep 2015 11:03:50 -0400 Date: Tue, 1 Sep 2015 17:03:43 +0200 From: Peter Zijlstra To: Byungchul Park Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, yuyang.du@intel.com Subject: Re: [PATCH v4 5/5] sched: add two functions for att(det)aching a task to(from) a cfs_rq Message-ID: <20150901150343.GT19282@twins.programming.kicks-ass.net> References: <1440069720-27038-1-git-send-email-byungchul.park@lge.com> <1440069720-27038-6-git-send-email-byungchul.park@lge.com> <20150820113516.GJ24261@byungchulpark-X58A-UD3R> <20150831152138.GO19282@twins.programming.kicks-ass.net> <20150901002849.GA30881@byungchulpark-X58A-UD3R> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150901002849.GA30881@byungchulpark-X58A-UD3R> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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