From patchwork Wed Mar 21 21:15:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dietmar Eggemann X-Patchwork-Id: 10300439 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E9C1460386 for ; Wed, 21 Mar 2018 21:15:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D56DD28329 for ; Wed, 21 Mar 2018 21:15:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C999B28D41; Wed, 21 Mar 2018 21:15:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3F9A928329 for ; Wed, 21 Mar 2018 21:15:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753454AbeCUVP3 (ORCPT ); Wed, 21 Mar 2018 17:15:29 -0400 Received: from foss.arm.com ([217.140.101.70]:59142 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753364AbeCUVP2 (ORCPT ); Wed, 21 Mar 2018 17:15:28 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B07521529; Wed, 21 Mar 2018 14:15:27 -0700 (PDT) Received: from [192.168.0.3] (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C949B3F592; Wed, 21 Mar 2018 14:15:23 -0700 (PDT) Subject: Re: [RFC PATCH 4/6] sched/fair: Introduce an energy estimation helper function To: Quentin Perret , Patrick Bellasi Cc: Juri Lelli , linux-kernel@vger.kernel.org, Peter Zijlstra , Thara Gopinath , linux-pm@vger.kernel.org, Morten Rasmussen , Chris Redpath , Valentin Schneider , "Rafael J . Wysocki" , Greg Kroah-Hartman , Vincent Guittot , Viresh Kumar , Todd Kjos , Joel Fernandes References: <20180320094312.24081-1-dietmar.eggemann@arm.com> <20180320094312.24081-5-dietmar.eggemann@arm.com> <20180321090430.GA6913@localhost.localdomain> <20180321122621.GA13951@e110439-lin> <20180321140235.GA2168@queper01-VirtualBox> From: Dietmar Eggemann Message-ID: <832b7772-aa83-5205-874b-06d1fcdc8b86@arm.com> Date: Wed, 21 Mar 2018 22:15:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20180321140235.GA2168@queper01-VirtualBox> Content-Language: en-US Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 03/21/2018 03:02 PM, Quentin Perret wrote: > On Wednesday 21 Mar 2018 at 12:26:21 (+0000), Patrick Bellasi wrote: >> On 21-Mar 10:04, Juri Lelli wrote: >>> Hi, >>> >>> On 20/03/18 09:43, Dietmar Eggemann wrote: >>>> From: Quentin Perret [...] >> Actually I think that this whole function can be written "just" as: >> >> ---8<--- >> unsigned long util = cpu_util_wake(cpu); >> >> if (cpu != dst_cpu) >> return util; >> >> return min(util + task_util(p), capacity_orig_of(cpu)); >> ---8<--- >> > > Yes this should be functionally equivalent. However, with your > suggestion you can potentially remove the task contribution from the > cpu_util in cpu_util_wake() and then add it back right after if > cpu==dst_cpu. This is sub-optimal and that's why I implemented things > slightly differently. But maybe this optimization really is too small to > justify the extra complexity involved ... What about we merge both functions by adding an additional 'int dst_cpu' parameter to cpu_util_wake() (only lightly tested, w/o util_est): --->8--- diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 65a1bead0773..4d4f104d5b3d 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -5860,11 +5860,11 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, } static inline unsigned long task_util(struct task_struct *p); -static unsigned long cpu_util_wake(int cpu, struct task_struct *p); +static unsigned long cpu_util_wake(int cpu, int dst_cpu, struct task_struct *p); static unsigned long capacity_spare_wake(int cpu, struct task_struct *p) { - return max_t(long, capacity_of(cpu) - cpu_util_wake(cpu, p), 0); + return max_t(long, capacity_of(cpu) - cpu_util_wake(cpu, -1, p), 0); } /* @@ -6384,16 +6384,22 @@ static inline unsigned long task_util(struct task_struct *p) * cpu_util_wake: Compute CPU utilization with any contributions from * the waking task p removed. */ -static unsigned long cpu_util_wake(int cpu, struct task_struct *p) +static unsigned long cpu_util_wake(int cpu, int dst_cpu, struct task_struct *p) { unsigned long util, capacity; /* Task has no contribution or is new */ - if (cpu != task_cpu(p) || !p->se.avg.last_update_time) + if ((cpu != task_cpu(p) && cpu != dst_cpu) || + dst_cpu == task_cpu(p) || !p->se.avg.last_update_time) return cpu_util(cpu); capacity = capacity_orig_of(cpu); - util = max_t(long, cpu_rq(cpu)->cfs.avg.util_avg - task_util(p), 0); + util = cpu_rq(cpu)->cfs.avg.util_avg; + + if (likely(dst_cpu != cpu)) + util = max_t(long, util - task_util(p), 0); + else + util += task_util(p); return (util >= capacity) ? capacity : util; } @@ -6409,30 +6415,6 @@ static inline int cpu_overutilized(int cpu) } /* - * Returns the util of "cpu" if "p" wakes up on "dst_cpu". - */ -static unsigned long cpu_util_next(int cpu, struct task_struct *p, int dst_cpu) -{ - unsigned long util = cpu_rq(cpu)->cfs.avg.util_avg; - unsigned long capacity = capacity_orig_of(cpu); - - /* - * If p is where it should be, or if it has no impact on cpu, there is - * not much to do. - */ - if ((task_cpu(p) == dst_cpu) || (cpu != task_cpu(p) && cpu != dst_cpu)) - goto clamp_util; - - if (dst_cpu == cpu) - util += task_util(p); - else - util = max_t(long, util - task_util(p), 0); - -clamp_util: - return (util >= capacity) ? capacity : util; -} - -/* * Disable WAKE_AFFINE in the case where task @p doesn't fit in the * capacity of either the waking CPU @cpu or the previous CPU @prev_cpu. * @@ -6488,7 +6470,7 @@ static unsigned long compute_energy(struct task_struct *p, int dst_cpu) for_each_freq_domain(fdom) { fdom_max_util = 0; for_each_cpu_and(cpu, &(fdom->span), cpu_online_mask) { - util = cpu_util_next(cpu, p, dst_cpu); + util = cpu_util_wake(cpu, dst_cpu, p); fdom_max_util = max(util, fdom_max_util); } @@ -6506,7 +6488,7 @@ static unsigned long compute_energy(struct task_struct *p, int dst_cpu) * busy time. */ for_each_cpu_and(cpu, &(fdom->span), cpu_online_mask) { - util = cpu_util_next(cpu, p, dst_cpu); + util = cpu_util_wake(cpu, dst_cpu, p); energy += cs->power * util / cs->cap; } }