Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 29 Jun 2018 11:45:25 -0700
From:   Joel Fernandes <joel@joelfernandes.org>
To:     Alessio Balsini <alessio.balsini@gmail.com>
Cc:     linux-kernel@vger.kernel.org, Juri Lelli <juri.lelli@redhat.com>,
        Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>,
        Luca Abeni <luca.abeni@santannapisa.it>,
        Claudio Scordino <claudio@evidence.eu.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Patrick Bellasi <patrick.bellasi@arm.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute
 dl-task information
Message-ID: <20180629184525.GA261771@joelaf.mtv.corp.google.com>
References: <20180629120947.14579-1-alessio.balsini@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180629120947.14579-1-alessio.balsini@gmail.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 29, 2018 at 02:09:47PM +0200, Alessio Balsini wrote:
> If the task calls sched_getattr() with SCHED_GETATTR_FLAGS_DL_ABSOLUTE
> flag set, the returned runtime and deadline parameters are, accordingly,
> the remaining runtime and the absolute deadline.
> 
> To return consistent data, the scheduler and rq times, as well as the
> task statistics, are updated.

The commit log should typically also explain the "why" not just the "how".
Think about someone doing a git log later and trying to find out why this was
added.

> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>
> Cc: Luca Abeni <luca.abeni@santannapisa.it>
> Cc: Claudio Scordino <claudio@evidence.eu.com>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Tested-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> Signed-off-by: Alessio Balsini <alessio.balsini@gmail.com>
> ---
> Having a precise means for measuring the execution time of a task at
> each activation is critical for real-time application design,
> development, and deployment. This allows for estimating the
> computational demand of the task at run-time, constituting the
> fundamental information over which the runtime parameter can be set when
> using the SCHED_DEADLINE policy. For example, one could set the runtime
> equal to the maximum observed execution time, or a proper percentile of
> its observed distribution (while a discussion of more complex WCET
> estimation techniques is out of scope here). Moreover, in dynamic
> workload scenarios, one needs to track the execution time changes, in
> order to possibly adapt the runtime parameter as needed.

All this should be in the commit log?

> However, on platforms with frequency switching capabilities, the typical
> way to perform execution time measurements for a task, based on sampling
> the CLOCK_THREAD_CPUTIME_ID clock, produces unreliable results due to
> the sporadic frequency switches that may happen between two
> measurements, and locking down the frequency is rarely a viable
> solution, anyway only acceptable during design/development, not for
> dynamic adaptations while the task is running.
> 
> Execution time measurements can be done by using the remaining runtime
> and absolute deadline instead, for SCHED_DEADLINE tasks. This is a
> better option because (i) it only accounts for the actual runtime of the
> task, and (ii) the runtime accounting is automatically normalized
> (scaled) with the CPU frequency (and capacity, for heterogeneous
> platforms).
> 
> 
> This solution preserves the ability to query for the absolute

"The solution presented in this patch"

> sched_{runtime, deadline} values of tasks other than itself, simplifying
> the development of a task hierarchy where a manager process can allocate
> the bandwidths of other deadline tasks in the system.

You didn't mention here you are attempting to fetch the remaining runtime of
the DL task. It almost sounds like you are saying here that the new flag will
fetch the configured runtime which it doesn't.

> 
> 
> The simplest way to measure the normalized duration C_ns of a piece of

"The simples way this can be used"

> deadline task that does not use bandwidth reclaiming is the following:
> 
> struct sched_attr s, e;
> uint64_t dl_misses;
> struct sched_attr curr_attr = {
>     [...]
>     sched_policy = SCHED_DEADLINE,
>     [...]
> };
> 
> sched_setattr(0, &curr_attr, 0);
> 
> sched_getattr(0, &s, ..., SCHED_GETATTR_FLAGS_DL_ABSOLUTE);
> /* calculations to be measured */
> sched_getattr(0, &e, ..., SCHED_GETATTR_FLAGS_DL_ABSOLUTE);
> 
> /* SCHED_DL periods within measurement, usually 0 */
> n_periods = (e.sched_deadline - s.sched_deadline) / s.sched_period;
> C_ns = s.sched_runtime - e.sched_runtime + n_periods * t.sched_runtime;


Overall I feel the description can be improved and made clear in conveying
the need for the patch in calculating the execution time. Also you didn't
even mention the usecase of low-latency audio. :-(

thanks!

- Joel