From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DA665C433ED for ; Mon, 12 Apr 2021 17:15:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B241E61206 for ; Mon, 12 Apr 2021 17:15:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243861AbhDLRP2 (ORCPT ); Mon, 12 Apr 2021 13:15:28 -0400 Received: from foss.arm.com ([217.140.110.172]:56306 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243788AbhDLRPZ (ORCPT ); Mon, 12 Apr 2021 13:15:25 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C7B6931B; Mon, 12 Apr 2021 10:15:06 -0700 (PDT) Received: from [10.57.31.161] (unknown [10.57.31.161]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 050E03F73B; Mon, 12 Apr 2021 10:15:03 -0700 (PDT) From: Pierre Gondois Subject: Re: [PATCH] sched/fair: use signed long when compute energy delta in eas To: Xuewen Yan Cc: Dietmar Eggemann , Quentin Perret , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Steven Rostedt , Benjamin Segall , Mel Gorman , Daniel Bristot de Oliveira , linux-kernel , Chunyan Zhang , Ryan Y References: <20210330052154.26861-1-xuewen.yan94@gmail.com> <34ce11ad-9c20-7ba7-90d8-4830725bf38a@arm.com> <1ebddd33-4666-1e6e-7788-a3fe28c9e99c@arm.com> Message-ID: Date: Mon, 12 Apr 2021 18:14:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi > > > > > > > > This patch-set is not significantly improving the execution time of > > > > feec(). The results we have so far are an improvement of 5-10% in > > > > execution time, with feec() being executed in < 10us. So the > gain is not > > > > spectacular. > > > > > > well， I meaned to cache all util value and compute energy with > caches, > > > when > > > (cpu==dst_cpu), use caches instead of updating util, and do not get > > > util with function: > > >  "effective_cpu_util()", to compute util with cache. > > > I add more parameters into pd_cache: > > > struct pd_cache { > > >         unsigned long util; > > >         unsigned long util_est; > > >         unsigned long util_cfs; > > >         unsigned long util_irq; > > >         unsigned long util_rt; > > >         unsigned long util_dl; > > >         unsigned long bw_dl; > > >         unsigned long freq_util; > > >         unsigned long nrg_util; > > > }; > > > In this way, it can avoid util update while feec. I tested with it, > > > and the negative delta disappeared. > > > Maybe this is not a good method, but it does work. > > If I understand correctly, you put all the fields used by > > core.c:effective_cpu_util() in the caches, allowing to have values not > > subject to updates. > Yes. > > core.c:effective_cpu_util() isn't only called from > > fair.c:compute_energy(). It is used in the cpufreq_schedutil.c and > > cpufreq_cooling.c (through core.c:sched_cpu_util()). > > Did you have to duplicate core.c:effective_cpu_util() to have a second > > version using the caches ? If yes, I think the function was meant to be > > unique so that all the utilization estimations go through the same path. > > > I defined a new function to distinguish it from the effective_cpu_util. > > > If your concern is to avoid negative delta, I think just bailing out > > when this happens should be sufficient. As shown in the last message, > > having a wrong placement should not happen that often, plus the prev_cpu > > should be used which should be ok. > In your patch, you didn't actually choose the prev_cpu. you return (-1); > > > If you want to cache the values, I think a stronger justification will > > be asked: this seems to be a big modification compared to the initial > > issue, knowing that another simpler solution is available (i.e. bailing > > out). I was not able to prove there was a significant gain in the > > find_energy_efficient_cpu() execution time, but I would be happy if you > > can, or if you find other arguments. > Yes, you are right, perhaps there is indeed no need for such a big > modification. > > Regards In fair.c:select_task_rq_fair(), if feec() returns a error (< 0), then prev_cpu is selected. I think it's better to still let feec() signal that something happened and let select_task_rq_fair() select prev_cpu by itself. Are you planning to submit a V2 with the bail out mechanism ? Regards