Received: by 2002:a25:868d:0:0:0:0:0 with SMTP id z13csp3390583ybk; Tue, 19 May 2020 03:33:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwS+J9IW5ZjQ7u9YZsxQYwZ2tUZSTJ6aNyGXKsJI86KPCps+yqiUG5ilyiLR1jrt+iZ87mD X-Received: by 2002:a17:906:dbcf:: with SMTP id yc15mr18319701ejb.176.1589884383342; Tue, 19 May 2020 03:33:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1589884383; cv=none; d=google.com; s=arc-20160816; b=hoFw8WUjcUjg0X5OtEMRv+v6mOMJTZRejPWa3zL97qcx6Zr/FGX829VlWRatU7l274 B2CPP7MpYEuQ07sfBu1ItYm3MOiIl5iV7H0thB56yyBwn1/lomZQX/T4fFAv0DvIoHky jfJmvuccQrOB8b4GQHGEC3/iOWeynY351q6PlJikEZ0qEnbJJeVGQe+ZPGAJPyCdYOaU zzgWZWtUcePTJo5Ay/Z64EqL6JBzxUHUIKDmPFSyYRNdArJnd/tCBrP8x8eK1kgRj+F3 I+AVWJbP/mbEG6q5xTlntAbHNgxanxeJnkWlsCMRuiL0gRrlBgrofgGvg8v7aNnczwt5 FQmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:to:subject; bh=3eSnRYWOh8b5N+s44MPB1VsRe/H30fyDasIO+rUa87A=; b=BzVEdQfgsmNsDZ0pCPbZbn070nFqNnF88sJS/e6rAKX+2XENsWXjqdB3HvHRt1RQAs F1hdhhLpObI4KPGohmVfpqkUNKxfFam/DsuCVKfELlkQWhA3oPMUi6CnVQqeKUQfKx1Q eGt6GBphkIQ0xv0nBQ3LwLDlRnIvZg3UnF8d4yNCs4Ac3tQc4UB+Xp6WZfS9cyFx0rFC 6yKuX/zRWyOrQ3OtB0fus/THSpu29DSEnH3Kv7REgvEEZnf9fnM5jKOKYK/KgcruLft8 LdJNIIXjq36WrKOmD0Vb3S72Tzy3C4I5++C/0FkhUu7UAPpsT4ape/rltaivXSDBFdzu Z/Bw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id oa21si7912902ejb.375.2020.05.19.03.32.39; Tue, 19 May 2020 03:33:03 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726949AbgESK2g (ORCPT + 99 others); Tue, 19 May 2020 06:28:36 -0400 Received: from foss.arm.com ([217.140.110.172]:58452 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725911AbgESK2f (ORCPT ); Tue, 19 May 2020 06:28:35 -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 060C2101E; Tue, 19 May 2020 03:28:35 -0700 (PDT) Received: from [192.168.0.7] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A7DA03F305; Tue, 19 May 2020 03:28:33 -0700 (PDT) Subject: Re: [PATCH v2] sched/pelt: sync util/runnable_sum with PELT window when propagating To: Vincent Guittot , mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, linux-kernel@vger.kernel.org References: <20200506155301.14288-1-vincent.guittot@linaro.org> From: Dietmar Eggemann Message-ID: Date: Tue, 19 May 2020 12:28:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200506155301.14288-1-vincent.guittot@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/05/2020 17:53, Vincent Guittot wrote: [...] > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 02f323b85b6d..df3923a65162 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -3441,52 +3441,46 @@ static inline void > update_tg_cfs_util(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cfs_rq *gcfs_rq) > { > long delta = gcfs_rq->avg.util_avg - se->avg.util_avg; > + /* > + * cfs_rq->avg.period_contrib can be used for both cfs_rq and se. > + * See ___update_load_avg() for details. > + */ > + u32 divider = LOAD_AVG_MAX - 1024 + cfs_rq->avg.period_contrib; Why not doing the assignment (like in update_tg_cfs_load()) after the next condition? Same question for update_tg_cfs_runnable(). [...] > static inline void > update_tg_cfs_runnable(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cfs_rq *gcfs_rq) > { > long delta = gcfs_rq->avg.runnable_avg - se->avg.runnable_avg; > + /* > + * cfs_rq->avg.period_contrib can be used for both cfs_rq and se. > + * See ___update_load_avg() for details. > + */ > + u32 divider = LOAD_AVG_MAX - 1024 + cfs_rq->avg.period_contrib; We know have 6 assignments like this in fair.c and 1 in pelt.c. Could this not be refactored by using something like this in pelt.h: +static inline u32 get_divider(struct sched_avg *avg) +{ + return LOAD_AVG_MAX - 1024 + avg->period_contrib; +} [...] > diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c > index b647d04d9c8b..1feff80e7e45 100644 > --- a/kernel/sched/pelt.c > +++ b/kernel/sched/pelt.c > @@ -237,6 +237,30 @@ ___update_load_sum(u64 now, struct sched_avg *sa, > return 1; > } > > +/* > + * When syncing *_avg with *_sum, we must take into account the current > + * position in the PELT segment otherwise the remaining part of the segment > + * will be considered as idle time whereas it's not yet elapsed and this will > + * generate unwanted oscillation in the range [1002..1024[. > + * > + * The max value of *_sum varies with the position in the time segment and is > + * equals to : > + * > + * LOAD_AVG_MAX*y + sa->period_contrib > + * > + * which can be simplified into: > + * > + * LOAD_AVG_MAX - 1024 + sa->period_contrib > + * > + * because LOAD_AVG_MAX*y == LOAD_AVG_MAX-1024 Isn't this rather '~' instead of '==', even for y^32 = 0.5 ? 47742 * 0.5^(1/32) ~ 47742 - 1024 Apart from that, LGTM Reviewed-by: Dietmar Eggemann