Date: Mon, 27 Mar 2017 09:43:42 +0200
From: Luca Abeni <luca.abeni@santannapisa.it>
To: Juri Lelli <juri.lelli@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>, linux-kernel@vger.kernel.org,
        Ingo Molnar <mingo@redhat.com>,
        Claudio Scordino <claudio@evidence.eu.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Tommaso Cucinotta <tommaso.cucinotta@sssup.it>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Joel Fernandes <joelaf@google.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [RFC v5 2/9] sched/deadline: improve the tracking of active
 utilization
Message-ID: <20170327094342.3c686827@luca>
In-Reply-To: <20170327071745.GA10289@e106622-lin>
References: <1490327582-4376-1-git-send-email-luca.abeni@santannapisa.it>
        <1490327582-4376-3-git-send-email-luca.abeni@santannapisa.it>
        <20170324132041.6fxayfe3id4af3n5@hirez.programming.kicks-ass.net>
        <20170324224715.4098dbfb@nowhere>
        <20170327071745.GA10289@e106622-lin>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3762
Lines: 83

Hi Juri,

On Mon, 27 Mar 2017 08:17:45 +0100
Juri Lelli <juri.lelli@arm.com> wrote:
[...]
> > > In general I feel it would be nice to have a state diagram
> > > included somewhere near these two functions. It would be nice to
> > > not have to dig out the PDF every time.  
> > 
> > Ok... Since I am not good at ascii art, would it be ok to add a
> > textual description? If yes, I'll add a comment like:
> > "
> > The utilization of a task is added to the runqueue's active
> > utilization when the task becomes active (is enqueued in the
> > runqueue), and is  
> 
> Is enqueued for the first time on a new period, maybe? It seems to be
> contradictory w.r.t. what below (if wakeup before 0 lag time)
> otherwise.
I think it should be "is enqueued in the runqueue and was previously
not active" (I did not write the "and was previously not active" to
avoid complicanting the sentence even more... But this
"simplification" was not a good idea :). The fact that this happens in a
new period or not is (in my understanding) irrelevant...


> > removed when the task becomes inactive. A task does not become
> > immediately inactive when it blocks, but becomes inactive at the so
> > called "0 lag time"; so, we setup the "inactive timer" to fire at
> > the "0 lag time". When the "inactive timer" fires, the task
> > utilization is removed from the runqueue's active utilization. If
> > the task wakes up again on the same runqueue before the "0 lag
> > time", the active utilization must not be changed and the "inactive
> > timer" must be cancelled. If the task wakes up again on a different
> > runqueue before the "0 lag time", then the task's utilization must
> > be removed from the previous runqueue's active utilization and must
> > be added to the new runqueue's active utilization.
> > In order to avoid races between a task waking up on a runqueue
> > while the "inactive timer" is running on a different CPU, the
> > "dl_non_contending" flag is used to indicate that a task is not on
> > a runqueue but is active (so, the flag is set when the task blocks
> > and is cleared when the "inactive timer" fires or when the task
> > wakes up). "
> > (if this is ok, where can I add this comment?)
> >   
> 
> Thanks for this Luca. Not sure it adds much to your text above, but we
> might want to consider adding something like below?
> 
> --->8---  
>            1st enqueue       +------------------+
>                              |                  |
>            +---------------->+ ACTIVEcontending |
>            |                 |                  |
>            |                 +----+------+------+
>            |                      |      ^
>            |                      |      |
>   +--------+-------+              |      |
>   |                |     dequeue  |      |  wakeup before
>   |    INACTIVE    |              |      |  0 lag time
>   |                |              |      |
>   +--------+-------+              |      |
>            ^                      |      |
>            |                      V      |
>            |                 +----+------+------+
>            |                 |                  |
>            +-----------------+ ACTIVEnonCONTEND |
>                              |                  |
>             0 lag time       +------------------+
>             elapsed
> --->8---  

I am not sure if introducing the "active non contending" name is a good
idea or not (see my previous email), but I am not the best person to
decide this... If people like this figure, I am more than happy to add
it :)
(but then maybe we can change "0 lag time elapsed" with "inactive timer
fires" and we can display in the figure the state of the
"dl_non_contending"/"inactive_timer_armed" flag)


			Thanks,
				Luca