From: Patrick Bellasi <patrick.bellasi@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Alessio Balsini <alessio.balsini@gmail.com>,
linux-kernel@vger.kernel.org,
Joel Fernandes <joel@joelfernandes.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>,
Ingo Molnar <mingo@redhat.com>
Subject: Re: [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information
Date: Mon, 23 Jul 2018 15:31:12 +0100 [thread overview]
Message-ID: <20180723143112.GC2683@e110439-lin> (raw)
In-Reply-To: <20180723141322.GZ2458@hirez.programming.kicks-ass.net>
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
next prev parent reply other threads:[~2018-07-23 14:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-29 12:09 [RFC PATCH] sched/deadline: sched_getattr() returns absolute dl-task information Alessio Balsini
2018-06-29 18:45 ` Joel Fernandes
2018-07-23 9:49 ` Peter Zijlstra
2018-07-23 12:49 ` Patrick Bellasi
2018-07-23 14:13 ` Peter Zijlstra
2018-07-23 14:31 ` Patrick Bellasi [this message]
2019-04-29 16:11 ` Alessio Balsini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180723143112.GC2683@e110439-lin \
--to=patrick.bellasi@arm.com \
--cc=alessio.balsini@gmail.com \
--cc=bristot@redhat.com \
--cc=claudio@evidence.eu.com \
--cc=joel@joelfernandes.org \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.abeni@santannapisa.it \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tommaso.cucinotta@santannapisa.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).