From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753640AbbHaPVq (ORCPT ); Mon, 31 Aug 2015 11:21:46 -0400 Received: from casper.infradead.org ([85.118.1.10]:41521 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753276AbbHaPVp (ORCPT ); Mon, 31 Aug 2015 11:21:45 -0400 Date: Mon, 31 Aug 2015 17:21:38 +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: <20150831152138.GO19282@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150820113516.GJ24261@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 Thu, Aug 20, 2015 at 08:35:16PM +0900, Byungchul Park wrote: > On Thu, Aug 20, 2015 at 08:22:00PM +0900, byungchul.park@lge.com wrote: > > + /* > > + * If it's !queued, then only when the task is sleeping it has a > > + * non-normalized vruntime, that is, when the task is being migrated > > + * it has a normailized vruntime. > > + */ > > i tried to change your XXX comment. i think it can be explaned like this. > don't you think so? i want to hear any opinions about this. > > > + if (p->state == TASK_RUNNING) > > + return true; --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -7943,11 +7943,10 @@ static inline bool vruntime_normalized(s return true; /* - * If it's !queued, then only when the task is sleeping it has a - * non-normalized vruntime, that is, when the task is being migrated - * it has a normalized vruntime. + * If it's !queued, sleeping tasks have a normalized vruntime, + * see dequeue_entity(). */ - if (p->state == TASK_RUNNING) + if (!p->se.on_rq) return true; return false; Does that make sense? I think using p->state for this is fragile, as we could be racy with any random blocking primitive that does set_current_state() _before_ actually calling into the scheduler.