Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031123AbdD0I7w (ORCPT ); Thu, 27 Apr 2017 04:59:52 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:33907 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030823AbdD0I7e (ORCPT ); Thu, 27 Apr 2017 04:59:34 -0400 MIME-Version: 1.0 In-Reply-To: <20170426222744.GA9616@wtj.duckdns.org> References: <20170424201344.GA14169@wtj.duckdns.org> <20170424201415.GB14169@wtj.duckdns.org> <20170426222744.GA9616@wtj.duckdns.org> From: Vincent Guittot Date: Thu, 27 Apr 2017 10:59:12 +0200 Message-ID: Subject: Re: [PATCH 1/2] sched/fair: Fix how load gets propagated from cfs_rq to its sched_entity To: Tejun Heo Cc: Ingo Molnar , Peter Zijlstra , linux-kernel , Linus Torvalds , Mike Galbraith , Paul Turner , Chris Mason , kernel-team@fb.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1276 Lines: 36 On 27 April 2017 at 00:27, Tejun Heo wrote: > Hello, Vincent. > > On Wed, Apr 26, 2017 at 06:14:17PM +0200, Vincent Guittot wrote: >> > + if (gcfs_rq->load.weight) { >> > + long shares = calc_cfs_shares(gcfs_rq, gcfs_rq->tg); >> > >> > + load = min(gcfs_rq->runnable_load_avg * >> > + shares / gcfs_rq->load.weight, shares); >> >> There is a unit problem above: >> runnable_load_avg and shares are not in the same range but >> runnable_load_avg and scale_load_down(gcfs_rq->load.weight) are so >> you should use >> gcfs_rq->runnable_load_avg * scale_load_down(shares) / >> scale_load_down(gcfs_rq->load.weight). > > But the only difference there is that we lose accuracy in calculation; > otherwise, the end results are the same, no? Yes the end result is the same, it was mainly to point out the range difference and explain why we need scale_load_down(shares) for the 2nd argument of min. This should also explain the warning issue you mentioned earlier > >> Hopefully both scale_load_down cancel between them >> But the min should be then tested with scale_load_down(shares) and not >> only shares > > Ah, that's right. The min should be against scaled down shares. > > Thanks. > > -- > tejun