Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932780AbcKJMp2 (ORCPT ); Thu, 10 Nov 2016 07:45:28 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34334 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932295AbcKJMp0 (ORCPT ); Thu, 10 Nov 2016 07:45:26 -0500 Date: Thu, 10 Nov 2016 13:45:19 +0100 From: luca abeni To: Juri Lelli Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Claudio Scordino , Steven Rostedt Subject: Re: [RFC v3 2/6] Improve the tracking of active utilisation Message-ID: <20161110134519.76d245c6@sweethome> In-Reply-To: <20161110123415.GJ16920@e106622-lin> References: <1477317998-7487-1-git-send-email-luca.abeni@unitn.it> <1477317998-7487-3-git-send-email-luca.abeni@unitn.it> <20161101164604.GB2769@ARMvm> <20161101224633.4e5ee0ca@utopia> <20161102033552.4d41c9ca@utopia> <20161110100428.GH16920@e106622-lin> <20161110115610.GI16920@e106622-lin> <20161110131558.7a9480b2@sweethome> <20161110123415.GJ16920@e106622-lin> Organization: university of trento X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2271 Lines: 53 On Thu, 10 Nov 2016 12:34:15 +0000 Juri Lelli wrote: [...] > > Ok; I'll think about some possible solution for this race... If I do > > not find any simple way to solve it, I'll add a "contending" flag, > > which allows to know if the inactive timer handler already executed > > or not. > > > > Right, this might help. > > Another thing that I was thinking of is whether we can use the return > value of hrtimer_try_to_cancel() to decide what to do: > > - if it returns 0 it means that the callback exectuted or the timer > was never set, so nothing to do (as in nothing to sub_running_bw from) > - if it returns 1 we succedeed, so we need to actively sub_running_bw > - if -1 we can assume that it will eventually do sub_running_bw() so > we don't need to care explicitly This is about what I did in the initial version of the patch, but I found some problems... I'll try to have another look at this approach. > Now I guess the problem is that the task can be migrated while his > inactive_timer is set (by select_task_rq_dl or by other classes load > balacing if setscheduled to a different class). Can't we store a back > reference to the rq from which the inactive_timer was queued and use > that to sub_running_bw() from? It seems that we might end up with some > "shadow" bandwidth, say when we do a wakeup migration, but maybe this > is something we can tolerate? Just thinking aloud. :) I'll think about this... > > BTW, talking about sched_dl_entity flags: I see there are three > > different int fields "dl_throttled, "dl_boosted" and "dl_yielded"; > > any reason for doing this instead of having a "dl_flags" field and > > setting its different bits when the entity is throttled, boosted or > > yielded? In other words: if I need this "contending" flag, should I > > add a new "dl_contending" field? > > > > I think you might want to add a clean-up patch to your series (or a > separate one) fixing the current situation, and the build on to adding > the new flag if needed. Ok, can do this. I just wanted to know if there is a reason for having different "dl_*" fields instead of a "flags" field (I do not know, maybe performance?) or if it is just an historical accident and this can be changed. Thanks, Luca