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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC961C433EF for ; Fri, 8 Apr 2022 08:57:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231701AbiDHI7o (ORCPT ); Fri, 8 Apr 2022 04:59:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230314AbiDHI7f (ORCPT ); Fri, 8 Apr 2022 04:59:35 -0400 Received: from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com [IPv6:2607:f8b0:4864:20::1133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F26B6FE436 for ; Fri, 8 Apr 2022 01:57:31 -0700 (PDT) Received: by mail-yw1-x1133.google.com with SMTP id 00721157ae682-2e5e9025c20so88366907b3.7 for ; Fri, 08 Apr 2022 01:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Hr6J2iPWEM1G7f/GWAb9UseXtfq7g0i2J/Jf0pGrEeA=; b=CfKyNLLdlzoSnVWbuL18jbZh3k0NYHmE7xz045BxLpwzVB9xiTBJj+vaXYCc60dVLe 37hsTlNWhWYDkm89uCYX1VpAU1EYe+M4MrAIaAwfRUVrG/yKVYc+GUE7PvjQPJr/NS88 94dgmwQaVU8clCz9YdZLWbBm6Rx23DefakRwti6+VHFBjUVpdRjWlJWruxeAGohZTyKq xOI0XYdcjaV7wG9ewnsuvFGDEh20mYLpakYUrLm2qu/DDt/ESy63epRA30Xw9VhwCYGp xWAVSOxBc07s8i7t6NYqZU5ApSCSI3p2LvH1A2HSfvXr4VAMJ1JGJunwVAnDp7Gk21oS zzCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Hr6J2iPWEM1G7f/GWAb9UseXtfq7g0i2J/Jf0pGrEeA=; b=cLeCBOPjUFDrPxd8T6XDpJIwALRga6/uuYCcAUc1JczNycWZYToinhA2YWR6vYgEIM BNgrK+q4vEyPmdCNplf/6NhmmVeauu3b/o8t1pfVHSHwTJq9LTnsph6c1nqvl05k2iwm nGIIZnYDxVgNm46eeWOOqOyWV3aAPadIAyo0EIJWPFZW6zvTliYpPjm/nwaCykUEK7US jPKisuaDWR8JEaw6gc3gZWjEtQXHVIRy6MEmlrKD8uGiSMUdvJygU1MDuh6tO3gsogX4 C7+wwrCDo+n+675fm1UVvTtdjaCv869DIgYrn7wsxt3QANTbiUXvIwJ54kG1eb6yArwO DFwA== X-Gm-Message-State: AOAM530ZFt+vIGT8mlxT/0p8WA7B2F3Ft2xg8d0nSXl6ciQcUBKOVqj2 idxocV+6PAumfYB9UirZuzKSl4ppFHXZ1gCKDN0IRzlz94E= X-Google-Smtp-Source: ABdhPJwsUiUt8HXafvzY8HWkiqUJqhCSoXJGUTMwW6nSVzoIY4h8nywnaQNGIe/XjW5dGwphFcZ4DS0lG+mTgRIwoxY= X-Received: by 2002:a0d:fa01:0:b0:2d6:595d:81d4 with SMTP id k1-20020a0dfa01000000b002d6595d81d4mr15578498ywf.86.1649408251125; Fri, 08 Apr 2022 01:57:31 -0700 (PDT) MIME-Version: 1.0 References: <20220204141941.GE4077@xsang-OptiPlex-9020> In-Reply-To: From: Vincent Guittot Date: Fri, 8 Apr 2022 10:57:19 +0200 Message-ID: Subject: Re: [sched/pelt] 2d02fa8cc2: stress-ng.pipeherd.ops_per_sec -9.7% regression To: Chen Yu Cc: kernel test robot , Peter Zijlstra , Dietmar Eggemann , Sachin Sant , LKML , lkp@lists.01.org, 0day robot , Huang Ying , feng.tang@intel.com, zhengjun.xing@linux.intel.com, fengwei.yin@intel.com, Aubrey Li , Chen Yu , tim.c.chen@intel.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 5 Apr 2022 at 16:23, Chen Yu wrote: > > On Mon, Apr 4, 2022 at 5:53 PM Vincent Guittot > wrote: > > > > On Fri, 1 Apr 2022 at 20:32, Chen Yu wrote: > > > > > > On Fri, Apr 1, 2022 at 12:17 AM Vincent Guittot > > > wrote: > > > > > > > > On Thu, 31 Mar 2022 at 16:19, Chen Yu wrote: > > > > > > > > > > Hi Vincent, > > > > > > > > > > On Wed, Feb 9, 2022 at 1:17 PM kernel test robot wrote: > > > > > > > > > > > > > > > > > > > > > > > > Greeting, > > > > > > > > > > > > FYI, we noticed a -9.7% regression of stress-ng.pipeherd.ops_per_sec due to commit: > > > > > > > > > > > > > > > > > > commit: 2d02fa8cc21a93da35cfba462bf8ab87bf2db651 ("sched/pelt: Relax the sync of load_sum with load_avg") > > > > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master > > > > > > > > > > > > in testcase: stress-ng > > > > > > on test machine: 128 threads 2 sockets Intel(R) Xeon(R) Platinum 8358 CPU @ 2.60GHz with 128G memory > > > > > > with following parameters: > > > > > > > > > > > > nr_threads: 100% > > > > > > testtime: 60s > > > > > > class: memory > > > > > > test: pipeherd > > > > > > cpufreq_governor: performance > > > > > > ucode: 0xd000280 > > > > > > > > > > > This week we have re-run the test result and it seems that this > > > > > regression is still there. > > > > > As we are evaluating whether this report is valid or if the > > > > > downgrading is expected, appreciated > > > > > if you could give suggestion on further steps: > > > > > > > > > > 1. If I understand correctly, > > > > > 2d02fa8cc21a93da35cfba462bf8ab87bf2db651 ("sched/pelt: Relax the sync > > > > > of load_sum with load_avg") > > > > > fixed the calculating of load_sum. Before this patch the > > > > > contribution part would be 'skipped' and caused the load_sum > > > > > to be lower than expected. > > > > > > > > Yes, you understand it correctly > > > > > > > > > 2. If above is true, after this patch, the load_sum becomes higher. Is > > > > > there a scenario that higher load_sum added to 1 cfs_rq brings > > > > > more imbalance between this group and other sched_group, thus > > > > > brings more task migration/wake up? (because in below perf result, > > > > > it seems that, with this patch applied, there are slightly more > > > > > take wake up) > > > > > > > > This change should not impact load balance as it only does comparison > > > > and I expect the load increase to happen on all cfs rq. > > > > The only place that could be impacted, would be wake_affine_weight() > > > > because it removes task load from previous cfs rq load before > > > > comparing. > > > > The task's load was not impacted by the underestimate which means that > > > > the load of prev cfs might be seen lower than current cfs after > > > > subtracting the task's load whereas both cfs rqs were similarly > > > > underestimated. > > > > Now the load of prev cfs rq is not underestimated and becomes > > > > comparable or slightly higher than the current cfs and the task > > > > migrate on current cfs instead of staying on prev one at wakeup > > > > > > > Could you please elaborate a little more on this scenario, since both current > > > and previous cfs rqs were underestimated, how could previous cfs rq has > > > lower load than the current one before applying this patch? > > > > > > Say, suppose the previous cfs rq has a load of L1, and current cfs rq has > > > a load of L2, the waken task has a load of h, then wake_affine_weight() > > > compares L1 - h with L2 + h , when L1 < L2 + 2h, the task will remain on > > > previous CPU. Since L1 and L2 were underestimated in the same scale, > > > I'm not quite sure how this patch would affect the choice between > > > prev and current CPU. > > > > Let's take the example of this_cpu load L1 = 0 and prev_cpu load L2 = > > 2h'+d. h' reflects h in the cpu load and d is a small delta load. The > > task will migrate if we have the condition below: > > > > h < 2h'-h+d > > > > With this patch, we assume that h' == h as we don't underestimate the > > load of cfs rqs anymore. The condition for migrating the task is : > > h < h+d > > And the task will migrate on this cpu as soon as there is a small load > > on prev_cpu in addition to the 2h. > > > > Without the patch, the load of cfs_rqs are underestimated which means > > that the task's load is underestimated in the cfs rq. This can be > > described as h' == h-U. U being the underestimated part. In this case > > the condition to migrate the task becomes: > > h < h-2U+d > > The task will migrate on this cpu is d is large enough to compensate > > the underestimation so we will migrate less often > > > I see. Thanks for this example! So in this scenario when previous CPU > has some higher load than the current CPU, without this patch applied, > the OS would have more chances to keep the task on the previous CPU, > thus less migration and less rq lock contention(according to the perf result). > I don't have a good idea in mind on how to deal with this case, except that > by disabling WA_WEIGHT or WA_BIAS(prefer 'this' CPU I suppose). I don't think that there is any good solution for this bench. It was taking advantage of the underestimated load_avg because it seems that it doesn't like to migrate on local cpu but on the other side, some benches will take advantage of this migration. Thanks Vincent > > -- > Thanks, > Chenyu From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============7596516404074679760==" MIME-Version: 1.0 From: Vincent Guittot To: lkp@lists.01.org Subject: Re: [sched/pelt] 2d02fa8cc2: stress-ng.pipeherd.ops_per_sec -9.7% regression Date: Fri, 08 Apr 2022 10:57:19 +0200 Message-ID: In-Reply-To: List-Id: --===============7596516404074679760== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Tue, 5 Apr 2022 at 16:23, Chen Yu wrote: > > On Mon, Apr 4, 2022 at 5:53 PM Vincent Guittot > wrote: > > > > On Fri, 1 Apr 2022 at 20:32, Chen Yu wrote: > > > > > > On Fri, Apr 1, 2022 at 12:17 AM Vincent Guittot > > > wrote: > > > > > > > > On Thu, 31 Mar 2022 at 16:19, Chen Yu wr= ote: > > > > > > > > > > Hi Vincent, > > > > > > > > > > On Wed, Feb 9, 2022 at 1:17 PM kernel test robot wrote: > > > > > > > > > > > > > > > > > > > > > > > > Greeting, > > > > > > > > > > > > FYI, we noticed a -9.7% regression of stress-ng.pipeherd.ops_pe= r_sec due to commit: > > > > > > > > > > > > > > > > > > commit: 2d02fa8cc21a93da35cfba462bf8ab87bf2db651 ("sched/pelt: = Relax the sync of load_sum with load_avg") > > > > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git= master > > > > > > > > > > > > in testcase: stress-ng > > > > > > on test machine: 128 threads 2 sockets Intel(R) Xeon(R) Platinu= m 8358 CPU @ 2.60GHz with 128G memory > > > > > > with following parameters: > > > > > > > > > > > > nr_threads: 100% > > > > > > testtime: 60s > > > > > > class: memory > > > > > > test: pipeherd > > > > > > cpufreq_governor: performance > > > > > > ucode: 0xd000280 > > > > > > > > > > > This week we have re-run the test result and it seems that this > > > > > regression is still there. > > > > > As we are evaluating whether this report is valid or if the > > > > > downgrading is expected, appreciated > > > > > if you could give suggestion on further steps: > > > > > > > > > > 1. If I understand correctly, > > > > > 2d02fa8cc21a93da35cfba462bf8ab87bf2db651 ("sched/pelt: Relax the = sync > > > > > of load_sum with load_avg") > > > > > fixed the calculating of load_sum. Before this patch the > > > > > contribution part would be 'skipped' and caused the load_sum > > > > > to be lower than expected. > > > > > > > > Yes, you understand it correctly > > > > > > > > > 2. If above is true, after this patch, the load_sum becomes highe= r. Is > > > > > there a scenario that higher load_sum added to 1 cfs_rq brings > > > > > more imbalance between this group and other sched_group, thus > > > > > brings more task migration/wake up? (because in below perf result, > > > > > it seems that, with this patch applied, there are slightly mo= re > > > > > take wake up) > > > > > > > > This change should not impact load balance as it only does comparis= on > > > > and I expect the load increase to happen on all cfs rq. > > > > The only place that could be impacted, would be wake_affine_weight() > > > > because it removes task load from previous cfs rq load before > > > > comparing. > > > > The task's load was not impacted by the underestimate which means t= hat > > > > the load of prev cfs might be seen lower than current cfs after > > > > subtracting the task's load whereas both cfs rqs were similarly > > > > underestimated. > > > > Now the load of prev cfs rq is not underestimated and becomes > > > > comparable or slightly higher than the current cfs and the task > > > > migrate on current cfs instead of staying on prev one at wakeup > > > > > > > Could you please elaborate a little more on this scenario, since both= current > > > and previous cfs rqs were underestimated, how could previous cfs rq h= as > > > lower load than the current one before applying this patch? > > > > > > Say, suppose the previous cfs rq has a load of L1, and current cfs rq= has > > > a load of L2, the waken task has a load of h, then wake_affine_weight= () > > > compares L1 - h with L2 + h , when L1 < L2 + 2h, the task will rema= in on > > > previous CPU. Since L1 and L2 were underestimated in the same scale, > > > I'm not quite sure how this patch would affect the choice between > > > prev and current CPU. > > > > Let's take the example of this_cpu load L1 =3D 0 and prev_cpu load L2 = =3D > > 2h'+d. h' reflects h in the cpu load and d is a small delta load. The > > task will migrate if we have the condition below: > > > > h < 2h'-h+d > > > > With this patch, we assume that h' =3D=3D h as we don't underestimate t= he > > load of cfs rqs anymore. The condition for migrating the task is : > > h < h+d > > And the task will migrate on this cpu as soon as there is a small load > > on prev_cpu in addition to the 2h. > > > > Without the patch, the load of cfs_rqs are underestimated which means > > that the task's load is underestimated in the cfs rq. This can be > > described as h' =3D=3D h-U. U being the underestimated part. In this c= ase > > the condition to migrate the task becomes: > > h < h-2U+d > > The task will migrate on this cpu is d is large enough to compensate > > the underestimation so we will migrate less often > > > I see. Thanks for this example! So in this scenario when previous CPU > has some higher load than the current CPU, without this patch applied, > the OS would have more chances to keep the task on the previous CPU, > thus less migration and less rq lock contention(according to the perf res= ult). > I don't have a good idea in mind on how to deal with this case, except th= at > by disabling WA_WEIGHT or WA_BIAS(prefer 'this' CPU I suppose). I don't think that there is any good solution for this bench. It was taking advantage of the underestimated load_avg because it seems that it doesn't like to migrate on local cpu but on the other side, some benches will take advantage of this migration. Thanks Vincent > > -- > Thanks, > Chenyu --===============7596516404074679760==--