Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1754661AbcLSXHT (ORCPT <rfc822;w@1wt.eu>);
        Mon, 19 Dec 2016 18:07:19 -0500
Received: from hera.aquilenet.fr ([141.255.128.1]:36189 "EHLO
        hera.aquilenet.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753474AbcLSXHQ (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 19 Dec 2016 18:07:16 -0500
Date: Tue, 20 Dec 2016 00:07:13 +0100
From: Samuel Thibault <samuel.thibault@ens-lyon.org>
To: Paul Turner <pjt@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] sched/fair: fix calc_cfs_shares fixed point arithmetics
Message-ID: <20161219230713.GD2895@var.home>
Mail-Followup-To: Samuel Thibault <samuel.thibault@ens-lyon.org>,
        Paul Turner <pjt@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@kernel.org>
References: <20161219224014.GA28238@var.home>
 <CAPM31RJOx-WsE9zRYRAkiNBN1rA4uXRSOwkWEPfyWtRCdFwdtQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAPM31RJOx-WsE9zRYRAkiNBN1rA4uXRSOwkWEPfyWtRCdFwdtQ@mail.gmail.com>
User-Agent: Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1111
Lines: 30

Paul Turner, on Mon 19 Dec 2016 14:44:38 -0800, wrote:
> On Mon, Dec 19, 2016 at 2:40 PM, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > 2159197d6677 ("sched/core: Enable increased load resolution on 64-bit kernels")
> >
> > exposed yet another miscalculation in calc_cfs_shares: MIN_SHARES is unscaled,
> > and must thus be scaled before being manipulated against "shares" amounts.
> 
> It's actually intentional that MIN_SHARES is un-scaled here, this is
> necessary to support the goal of sub-partitioning groups with small
> shares.

Uh? you mean it's normal that MIN_SHARES is here compared as such
against "shares" while e.g. in sched_group_set_shares or effective_load
it is scaled before comparing with "shares"?

> E.g.  A group with shares=2 and 5 threads will internally provide 2048
> units of weight for the load-balancer to account for their
> distribution.

But here "shares" is already scaled, so

> > -       if (shares < MIN_SHARES)
> > -               shares = MIN_SHARES;
...
> >         return shares;

This will only make sure that the returned shares is 2, not 2048.

Samuel