[RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Alessio Balsini]

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.

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.

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
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.


The simplest way to measure the normalized duration C_ns of a piece of
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;

 include/uapi/linux/sched.h | 12 +++++++++++-
 kernel/sched/core.c        |  4 ++--
 kernel/sched/deadline.c    | 34 +++++++++++++++++++++++++++++++---
 kernel/sched/sched.h       |  2 +-
 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 22627f80063e..cf290d35685e 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -45,7 +45,17 @@
 #define SCHED_RESET_ON_FORK     0x40000000
 
 /*
- * For the sched_{set,get}attr() calls
+ * For the sched_getattr() call:
+ * - DL_ABSOLUTE: returns the current absolute deadline and remaining runtime,
+ *   instead of the sched_runtime and sched_deadline values.
+ */
+#define SCHED_GETATTR_FLAGS_DL_ABSOLUTE	0x01
+
+#define SCHED_GETATTR_FLAGS_ALL ( \
+			SCHED_GETATTR_FLAGS_DL_ABSOLUTE)
+
+/*
+ * For the struct sched_attr's sched_flags
  */
 #define SCHED_FLAG_RESET_ON_FORK	0x01
 #define SCHED_FLAG_RECLAIM		0x02
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 78d8facba456..40a172200147 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4729,7 +4729,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
 	int retval;
 
 	if (!uattr || pid < 0 || size > PAGE_SIZE ||
-	    size < SCHED_ATTR_SIZE_VER0 || flags)
+	    size < SCHED_ATTR_SIZE_VER0 || flags & ~SCHED_GETATTR_FLAGS_ALL)
 		return -EINVAL;
 
 	rcu_read_lock();
@@ -4746,7 +4746,7 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
 	if (p->sched_reset_on_fork)
 		attr.sched_flags |= SCHED_FLAG_RESET_ON_FORK;
 	if (task_has_dl_policy(p))
-		__getparam_dl(p, &attr);
+		__getparam_dl(p, &attr, flags);
 	else if (task_has_rt_policy(p))
 		attr.sched_priority = p->rt_priority;
 	else
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index fbfc3f1d368a..f75a4169cd47 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2568,13 +2568,41 @@ void __setparam_dl(struct task_struct *p, const struct sched_attr *attr)
 	dl_se->dl_density = to_ratio(dl_se->dl_deadline, dl_se->dl_runtime);
 }
 
-void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
+void __getparam_dl(struct task_struct *p, struct sched_attr *attr,
+		   unsigned int flags)
 {
 	struct sched_dl_entity *dl_se = &p->dl;
 
 	attr->sched_priority = p->rt_priority;
-	attr->sched_runtime = dl_se->dl_runtime;
-	attr->sched_deadline = dl_se->dl_deadline;
+
+	if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
+		/*
+		 * If the task is not running, its runtime is already
+		 * properly accounted. Otherwise, update clocks and the
+		 * statistics for the task.
+		 */
+		if (task_running(task_rq(p), p)) {
+			struct rq_flags rf;
+			struct rq *rq;
+
+			rq = task_rq_lock(p, &rf);
+			sched_clock_tick();
+			update_rq_clock(rq);
+			task_tick_dl(rq, p, 0);
+			task_rq_unlock(rq, p, &rf);
+		}
+
+		/*
+		 * If the task is throttled, this value could be negative,
+		 * but sched_runtime is unsigned.
+		 */
+		attr->sched_runtime = dl_se->runtime <= 0 ? 0 : dl_se->runtime;
+		attr->sched_deadline = dl_se->deadline;
+	} else {
+		attr->sched_runtime = dl_se->dl_runtime;
+		attr->sched_deadline = dl_se->dl_deadline;
+	}
+
 	attr->sched_period = dl_se->dl_period;
 	attr->sched_flags = dl_se->flags;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6601baf2361c..25892cd502aa 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -310,7 +310,7 @@ extern int  sched_dl_global_validate(void);
 extern void sched_dl_do_global(void);
 extern int  sched_dl_overflow(struct task_struct *p, int policy, const struct sched_attr *attr);
 extern void __setparam_dl(struct task_struct *p, const struct sched_attr *attr);
-extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr);
+extern void __getparam_dl(struct task_struct *p, struct sched_attr *attr, unsigned int flags);
 extern bool __checkparam_dl(const struct sched_attr *attr);
 extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
 extern int  dl_task_can_attach(struct task_struct *p, const struct cpumask *cs_cpus_allowed);
-- 
2.17.1

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Joel Fernandes]

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
 

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Peter Zijlstra]

On Fri, Jun 29, 2018 at 02:09:47PM +0200, Alessio Balsini wrote:

Joel nailed it wrt the Changelog, that needs improvement.


> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index fbfc3f1d368a..f75a4169cd47 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2568,13 +2568,41 @@ void __setparam_dl(struct task_struct *p, const struct sched_attr *attr)
>  	dl_se->dl_density = to_ratio(dl_se->dl_deadline, dl_se->dl_runtime);
>  }
>  
> -void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
> +void __getparam_dl(struct task_struct *p, struct sched_attr *attr,
> +		   unsigned int flags)
>  {
>  	struct sched_dl_entity *dl_se = &p->dl;
>  
>  	attr->sched_priority = p->rt_priority;
> -	attr->sched_runtime = dl_se->dl_runtime;
> -	attr->sched_deadline = dl_se->dl_deadline;
> +
> +	if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
> +		/*
> +		 * If the task is not running, its runtime is already
> +		 * properly accounted. Otherwise, update clocks and the
> +		 * statistics for the task.
> +		 */
> +		if (task_running(task_rq(p), p)) {
> +			struct rq_flags rf;
> +			struct rq *rq;
> +
> +			rq = task_rq_lock(p, &rf);
> +			sched_clock_tick();

This isn't required here. The reason it is used elsewhere is because
those are interrupts, but this is a system call, the clock state should
be good.

> +			update_rq_clock(rq);
> +			task_tick_dl(rq, p, 0);

Do we really want task_tick_dl() here, or update_curr_dl()? Also, who
says the task still is dl ? :-)

> +			task_rq_unlock(rq, p, &rf);
> +		}
> +
> +		/*
> +		 * If the task is throttled, this value could be negative,
> +		 * but sched_runtime is unsigned.
> +		 */
> +		attr->sched_runtime = dl_se->runtime <= 0 ? 0 : dl_se->runtime;
> +		attr->sched_deadline = dl_se->deadline;

This is all very racy..

Even if the task wasn't running when you did the task_running() test, it
could be running now. And if it was running, it might not be running
anymore by the time you've acquired the rq->lock.

On 32bit reading these numbers without locks is broken to boot. And even
on 64bit, I suppose you can a consistent snapshot of runtime and
deadline together, which isn't possible without the locks.

And of course, by the time we get back to userspace, the returned values
will be out-of-date anyway. But that isn't to be helped I suppose.


> +	} else {
> +		attr->sched_runtime = dl_se->dl_runtime;
> +		attr->sched_deadline = dl_se->dl_deadline;
> +	}
> +
>  	attr->sched_period = dl_se->dl_period;
>  	attr->sched_flags = dl_se->flags;
>  }

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Patrick Bellasi]

On 23-Jul 11:49, Peter Zijlstra wrote:

[...]

> > -void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
> > +void __getparam_dl(struct task_struct *p, struct sched_attr *attr,
> > +		   unsigned int flags)
> >  {
> >  	struct sched_dl_entity *dl_se = &p->dl;
> >  
> >  	attr->sched_priority = p->rt_priority;
> > -	attr->sched_runtime = dl_se->dl_runtime;
> > -	attr->sched_deadline = dl_se->dl_deadline;
> > +
> > +	if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
> > +		/*
> > +		 * If the task is not running, its runtime is already
> > +		 * properly accounted. Otherwise, update clocks and the
> > +		 * statistics for the task.
> > +		 */
> > +		if (task_running(task_rq(p), p)) {
> > +			struct rq_flags rf;
> > +			struct rq *rq;
> > +
> > +			rq = task_rq_lock(p, &rf);
> > +			sched_clock_tick();
> 
> This isn't required here. The reason it is used elsewhere is because
> those are interrupts, but this is a system call, the clock state should
> be good.
> 
> > +			update_rq_clock(rq);
> > +			task_tick_dl(rq, p, 0);
> 
> Do we really want task_tick_dl() here, or update_curr_dl()?

I think this was to cover the case of a syscall being called while the
task is running and we are midway between two ticks...

> Also, who says the task still is dl ? :-)

Good point, but what should be the rule in general for these cases?

We already have:

   SYSCALL_DEFINE4(sched_getattr())
       ....
       if (task_has_dl_policy(p))
            __getparam_dl(p, &attr);

which is also potentially racy, isn't it?

In this syscall we don't even use get_task_struct()... as we do for
example in sched_setaffinity()...

What should we do if, in the middle of such a  syscall, someone else
demoted the task to a different class?

Should we fail the syscall with a specific error?

Or just make the syscall return the most updated metrics for all the
scheduling classes since we cannot grant the user anything about what
the task will be once we return to userspace?


> > +			task_rq_unlock(rq, p, &rf);
> > +		}
> > +
> > +		/*
> > +		 * If the task is throttled, this value could be negative,
> > +		 * but sched_runtime is unsigned.
> > +		 */
> > +		attr->sched_runtime = dl_se->runtime <= 0 ? 0 : dl_se->runtime;
> > +		attr->sched_deadline = dl_se->deadline;
> 
> This is all very racy..
> 
> Even if the task wasn't running when you did the task_running() test, it
> could be running now. And if it was running, it might not be running
> anymore by the time you've acquired the rq->lock.

Which means we should use something like:

   if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
        /* Lock the task and the RQ before any other check and upate */
        rq = task_rq_lock(p, &rf);

        /* Check the task is still DL ?*/

        /* Update task stats */

        task_rq_unlock(rq, p, &rf);
   }

right?

If that's better, then we should probably even better move the
task_rq_lock at the beginning of SYSCALL_DEFINE4(sched_getattr()) ?

> On 32bit reading these numbers without locks is broken to boot. And even
> on 64bit, I suppose you can a consistent snapshot of runtime and
> deadline together, which isn't possible without the locks.

Indeed... I think we really need the lock and, likely, to place it at
the beginning of the syscall...

> And of course, by the time we get back to userspace, the returned values
> will be out-of-date anyway. But that isn't to be helped I suppose.

Yes, but that's always kind-of implied by syscall returning kernel
metrics, isn't it?

> > +	} else {
> > +		attr->sched_runtime = dl_se->dl_runtime;
> > +		attr->sched_deadline = dl_se->dl_deadline;
> > +	}
> > +
> >  	attr->sched_period = dl_se->dl_period;
> >  	attr->sched_flags = dl_se->flags;
> >  }

-- 
#include <best/regards.h>

Patrick Bellasi

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Peter Zijlstra]

On Mon, Jul 23, 2018 at 01:49:46PM +0100, Patrick Bellasi wrote:
> On 23-Jul 11:49, Peter Zijlstra wrote:
> 
> [...]
> 
> > > -void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
> > > +void __getparam_dl(struct task_struct *p, struct sched_attr *attr,
> > > +		   unsigned int flags)
> > >  {
> > >  	struct sched_dl_entity *dl_se = &p->dl;
> > >  
> > >  	attr->sched_priority = p->rt_priority;
> > > -	attr->sched_runtime = dl_se->dl_runtime;
> > > -	attr->sched_deadline = dl_se->dl_deadline;
> > > +
> > > +	if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
> > > +		/*
> > > +		 * If the task is not running, its runtime is already
> > > +		 * properly accounted. Otherwise, update clocks and the
> > > +		 * statistics for the task.
> > > +		 */
> > > +		if (task_running(task_rq(p), p)) {
> > > +			struct rq_flags rf;
> > > +			struct rq *rq;
> > > +
> > > +			rq = task_rq_lock(p, &rf);
> > > +			sched_clock_tick();
> > > +			update_rq_clock(rq);
> > > +			task_tick_dl(rq, p, 0);
> > 
> > Do we really want task_tick_dl() here, or update_curr_dl()?
> 
> I think this was to cover the case of a syscall being called while the
> task is running and we are midway between two ticks...

Sure, I know what it's there for, just saying that update_curr_dl()
would've updated the accounting as well. Calling tick stuff from !tick
context is a wee bit dodgy.

> > Also, who says the task still is dl ? :-)
> 
> Good point, but what should be the rule in general for these cases?
> 
> We already have:
> 
>    SYSCALL_DEFINE4(sched_getattr())
>        ....
>        if (task_has_dl_policy(p))
>             __getparam_dl(p, &attr);
> 
> which is also potentially racy, isn't it?

Yes, but only in so far as that the whole syscall is racy
per-definition. EVen if we'd lock the rq and get the absolute accurate
values, everything can change the moment we release the locks and return
to userspace again.

> Or just make the syscall return the most updated metrics for all the
> scheduling classes since we cannot grant the user anything about what
> the task will be once we return to userspace?

This.

> > > +			task_rq_unlock(rq, p, &rf);
> > > +		}
> > > +
> > > +		/*
> > > +		 * If the task is throttled, this value could be negative,
> > > +		 * but sched_runtime is unsigned.
> > > +		 */
> > > +		attr->sched_runtime = dl_se->runtime <= 0 ? 0 : dl_se->runtime;
> > > +		attr->sched_deadline = dl_se->deadline;
> > 
> > This is all very racy..
> > 
> > Even if the task wasn't running when you did the task_running() test, it
> > could be running now. And if it was running, it might not be running
> > anymore by the time you've acquired the rq->lock.
> 
> Which means we should use something like:
> 
>    if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
>         /* Lock the task and the RQ before any other check and upate */
>         rq = task_rq_lock(p, &rf);
> 
>         /* Check the task is still DL ?*/
> 
>         /* Update task stats */
> 
>         task_rq_unlock(rq, p, &rf);
>    }
> 
> right?

Yeah, something along those lines.

> If that's better, then we should probably even better move the
> task_rq_lock at the beginning of SYSCALL_DEFINE4(sched_getattr()) ?

Hurm.. yes, we should probably have the has_dl_policy test under the
lock too. Which is really annoying, because this basically turns a
lockless syscall into locked one.

Another method would be to have __getparam_dl() 'fail' and retry if it
finds !has_dl_policy() once we have the lock. That would retain the
lockless nature for all current use-cases and only incur the locking
overhead for this new case.

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Patrick Bellasi]

On 23-Jul 16:13, Peter Zijlstra wrote:
> On Mon, Jul 23, 2018 at 01:49:46PM +0100, Patrick Bellasi wrote:
> > On 23-Jul 11:49, Peter Zijlstra wrote:
> > 
> > [...]
> > 
> > > > -void __getparam_dl(struct task_struct *p, struct sched_attr *attr)
> > > > +void __getparam_dl(struct task_struct *p, struct sched_attr *attr,
> > > > +		   unsigned int flags)
> > > >  {
> > > >  	struct sched_dl_entity *dl_se = &p->dl;
> > > >  
> > > >  	attr->sched_priority = p->rt_priority;
> > > > -	attr->sched_runtime = dl_se->dl_runtime;
> > > > -	attr->sched_deadline = dl_se->dl_deadline;
> > > > +
> > > > +	if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
> > > > +		/*
> > > > +		 * If the task is not running, its runtime is already
> > > > +		 * properly accounted. Otherwise, update clocks and the
> > > > +		 * statistics for the task.
> > > > +		 */
> > > > +		if (task_running(task_rq(p), p)) {
> > > > +			struct rq_flags rf;
> > > > +			struct rq *rq;
> > > > +
> > > > +			rq = task_rq_lock(p, &rf);
> > > > +			sched_clock_tick();
> > > > +			update_rq_clock(rq);
> > > > +			task_tick_dl(rq, p, 0);
> > > 
> > > Do we really want task_tick_dl() here, or update_curr_dl()?
> > 
> > I think this was to cover the case of a syscall being called while the
> > task is running and we are midway between two ticks...
> 
> Sure, I know what it's there for, just saying that update_curr_dl()
> would've updated the accounting as well. Calling tick stuff from !tick
> context is a wee bit dodgy.

Right, I think it depends on how much we want to be "precise" in closing
a control loop with user-space.

On Android we have ticks every 3-4ms, I'm wondering if this maximum
"latency" on measuring the remaining run-time can introduce a too big
error for certain applications...

Alessio: you have an interesting low-latency audio use-case on hand,
do you think we can tolerate a 4ms error in remaining run-time
readings?

[...]

> > Which means we should use something like:
> > 
> >    if (flags & SCHED_GETATTR_FLAGS_DL_ABSOLUTE) {
> >         /* Lock the task and the RQ before any other check and upate */
> >         rq = task_rq_lock(p, &rf);
> > 
> >         /* Check the task is still DL ?*/
> > 
> >         /* Update task stats */
> > 
> >         task_rq_unlock(rq, p, &rf);
> >    }
> > 
> > right?
> 
> Yeah, something along those lines.
> 
> > If that's better, then we should probably even better move the
> > task_rq_lock at the beginning of SYSCALL_DEFINE4(sched_getattr()) ?
> 
> Hurm.. yes, we should probably have the has_dl_policy test under the
> lock too. Which is really annoying, because this basically turns a
> lockless syscall into locked one.

Indeed...

> Another method would be to have __getparam_dl() 'fail' and retry if it
> finds !has_dl_policy() once we have the lock. That would retain the
> lockless nature for all current use-cases and only incur the locking
> overhead for this new case.

... right, this is actually the best solution to have a bit more
guarantees for the new DL control scenarios without affecting existing
ones!

-- 
#include <best/regards.h>

Patrick Bellasi

Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information

[Alessio Balsini]