From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752215AbbHJLrN (ORCPT ); Mon, 10 Aug 2015 07:47:13 -0400 Received: from casper.infradead.org ([85.118.1.10]:37572 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750965AbbHJLrK (ORCPT ); Mon, 10 Aug 2015 07:47:10 -0400 Date: Mon, 10 Aug 2015 13:47:06 +0200 From: Peter Zijlstra To: byungchul.park@lge.com Cc: mingo@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] sched: sync with the prev cfs when changing cgroup within a cpu Message-ID: <20150810114706.GA16853@twins.programming.kicks-ass.net> References: <1439186939-27992-1-git-send-email-byungchul.park@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439186939-27992-1-git-send-email-byungchul.park@lge.com> 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 Mon, Aug 10, 2015 at 03:08:59PM +0900, byungchul.park@lge.com wrote: > From: Byungchul Park > > current code seems to be wrong with cfs_rq->blocked_load_avg when changing > a task's cgroup(=cfs_rq) to another. i tested with "echo pid > cgroup" and > found that cfs_rq->blocked_load_avg became larger and larger whenever i > changed a cgroup to another again and again. > > it is possible to move between groups within a cpu, and each cfs_rq is > tracking its own blocked load. so we have to sync se's average load with > both *prev* cfs_rq and next cfs_rq when changing its group. > > in addition, "#ifdef CONFIG_SMP" is removed becasuse we need to sync a > se's load with its cfs_rq even in the case of !SMP. remember it is possible > to move between groups in *a* cpu. > > i also removed some comments mentioning migration_task_rq_fair(). > migration_task_rq_fair() can be called in three cases. and in each case, > both decay counter and blocked load are already considered. so we > don't need to consider these in task_move_group_fair() at all. > > 1. the wake-up migration case > enqueue_entity_load_avg() makes se->avg.decay_count zero after applying it. > and it will be woken up soon so we don't need to add its load to > cfs_rq->blocked_load_avg. > > 2. the fork balancing case > se->avg.decay_count is initialized in __sched_fork() to zero. and > wake_up_new_task() calls activate_task() with flag = 0 so that > enqueue_entity_load_avg() can omit adding its load to > cfs_rq->blocked_load_avg, and it will be woken up soon. > > 3. the rq migration case (not wake up case) > the target task is already on rq, so we don't need to consider both its > decay counter and blocked load in this case. > > Signed-off-by: Byungchul Park What code is this against? Please look at current code and try again.