Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752076AbdHBN1x (ORCPT ); Wed, 2 Aug 2017 09:27:53 -0400 Received: from foss.arm.com ([217.140.101.70]:53754 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751186AbdHBN1w (ORCPT ); Wed, 2 Aug 2017 09:27:52 -0400 References: <20170802131002.31576-1-brendan.jackman@arm.com> <20170802132405.z5gvut7ecaygbhvy@hirez.programming.kicks-ass.net> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Brendan Jackman To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, Joel Fernandes , Andres Oportus , Dietmar Eggemann , Vincent Guittot , Josef Bacik , Morten Rasmussen Subject: Re: [PATCH] sched/fair: Sync task util before slow-path wakeup In-reply-to: <20170802132405.z5gvut7ecaygbhvy@hirez.programming.kicks-ass.net> Date: Wed, 02 Aug 2017 14:27:45 +0100 Message-ID: <8760e6rtf2.fsf@arm.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2354 Lines: 69 On Wed, Aug 02 2017 at 13:24, Peter Zijlstra wrote: > On Wed, Aug 02, 2017 at 02:10:02PM +0100, Brendan Jackman wrote: >> We use task_util in find_idlest_group via capacity_spare_wake. This >> task_util is updated in wake_cap. However wake_cap is not the only >> reason for ending up in find_idlest_group - we could have been sent >> there by wake_wide. So explicitly sync the task util with prev_cpu >> when we are about to head to find_idlest_group. >> >> We could simply do this at the beginning of >> select_task_rq_fair (i.e. irrespective of whether we're heading to >> select_idle_sibling or find_idlest_group & co), but I didn't want to >> slow down the select_idle_sibling path more than necessary. >> >> Don't do this during fork balancing, we won't need the task_util and >> we'd just clobber the last_update_time, which is supposed to be 0. > > So I remember Morten explicitly not aging util of tasks on wakeup > because the old util was higher and better representative of what the > new util would be, or something along those lines. > > Morten? > >> Signed-off-by: Brendan Jackman >> Cc: Dietmar Eggemann >> Cc: Vincent Guittot >> Cc: Josef Bacik >> Cc: Ingo Molnar >> Cc: Morten Rasmussen >> Cc: Peter Zijlstra >> --- >> kernel/sched/fair.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index c95880e216f6..62869ff252b4 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -5913,6 +5913,14 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int sd_flag, int wake_f >> new_cpu = cpu; >> } >> >> + if (sd && !(sd_flag & SD_BALANCE_FORK)) >> + /* >> + * We're going to need the task's util for capacity_spare_wake >> + * in select_idlest_group. Sync it up to prev_cpu's >> + * last_update_time. >> + */ >> + sync_entity_load_avg(&p->se); >> + > > That has missing {} OK. Also just noticed it refers to "select_idlest_group", will change to "find_idlest_group". > > >> if (!sd) { >> pick_cpu: > > And if this patch lives, can you please fix up that broken label indent? Sure. >> if (sd_flag & SD_BALANCE_WAKE) /* XXX always ? */ Cheers, Brendan