linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Bristot de Oliveira <bristot@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Juri Lelli <juri.lelli@arm.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-rt-users <linux-rt-users@vger.kernel.org>
Subject: Re: [PATCH V2 3/3] sched/deadline: Tracepoints for deadline scheduler
Date: Tue, 29 Mar 2016 14:37:18 -0300	[thread overview]
Message-ID: <56FABD4E.3080008@redhat.com> (raw)
In-Reply-To: <20160329115700.40acb336@gandalf.local.home>



On 03/29/2016 12:57 PM, Steven Rostedt wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
>> > On Mon, Mar 28, 2016 at 01:50:51PM -0300, Daniel Bristot de Oliveira wrote:
>>> > > @@ -733,7 +738,9 @@ static void update_curr_dl(struct rq *rq)
>>> > >  
>>> > >  throttle:
>>> > >  	if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) {
>>> > > +		trace_sched_deadline_yield(&rq->curr->dl);
>>> > >  		dl_se->dl_throttled = 1;
>>> > > +		trace_sched_deadline_throttle(dl_se);  
>> > 
>> > This is just really very sad.
> I agree. This should be a single tracepoint here. Especially since it
> seems that dl_se == &rq->curr->dl :-)
> 
> But perhaps we should add that generic sys_yield() tracepoint, to be
> able to see that the task was throttled because of a yield call.
> 
> We still want to see a task yield, and then throttle because of it. The
> deadline/runtime should reflect the information correctly.

The above tracepoints are conditional, if dl_se->dl_yielded, only the
yield tracepoint will happen. If !dl_se->dl_yielded, only the throttle
tracepoint will happen.

We can try to join the sched_deadline_(yield|throttle|block) on a single
tracepoint, but IMHO having them separated is more intuitive for users.

> Sure, we'll probably want to figure out a better way to see deadline
> tasks blocked. Probably can see that from sched switch though, as it
> would be in the blocked state as it scheduled out.

We can guess that in the sched switch, but it currently does not show
deadline specific information (deadline, runtime, now (in the timer used
by the scheduler), and they are relevant in the analysis of deadline tasks.

> 
> Hmm, I probably could add tracing infrastructure that would let us
> extend existing tracepoints. That is, without modifying sched_switch,
> we could add a new tracepoint that when enabled, would attach itself to
> the sched_switch tracepoint and record different information. Like a
> special sched_switch_deadline tracepoint, that would record the existing
> runtime,deadline and period for deadline tasks. It wont add more
> tracepoints into the core scheduler, but use the existing one.

You can display the joined version of sched deadline
(yield|throttle|block) tracepoint, but IMHO this will just turn things
more complex than they need to be, and will possibly add overhead to the
sched_switch tracepoint, that is more used by non-deadline users than by
deadline users.

Moreover, deadline users will probably want to see only the deadline
data, and have to use complex enabling/filtering options is really
really not intuitive for users (that are not kernel developers).

  parent reply	other threads:[~2016-03-29 17:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-28 16:50 [PATCH V2 0/3] Tracepoints for deadline scheduler Daniel Bristot de Oliveira
2016-03-28 16:50 ` [PATCH V2 1/3] tracing: Add __print_ns_to_secs() and __print_ns_without_secs() helpers Daniel Bristot de Oliveira
2016-03-28 16:50 ` [PATCH V2 2/3] sched: Move deadline container_of() helper functions into sched.h Daniel Bristot de Oliveira
2016-03-28 16:50 ` [PATCH V2 3/3] sched/deadline: Tracepoints for deadline scheduler Daniel Bristot de Oliveira
2016-03-29 15:16   ` Peter Zijlstra
2016-03-29 15:57     ` Steven Rostedt
2016-03-29 16:04       ` Peter Zijlstra
2016-03-29 17:10         ` Steven Rostedt
2016-03-29 20:11           ` Peter Zijlstra
2016-03-29 20:29             ` Steven Rostedt
2016-03-29 20:46               ` Peter Zijlstra
2016-03-29 20:57               ` Daniel Bristot de Oliveira
2016-03-29 21:03                 ` Peter Zijlstra
2016-03-29 21:49                   ` Steven Rostedt
2016-03-29 17:37       ` Daniel Bristot de Oliveira [this message]
2016-03-29 18:10         ` Steven Rostedt
2016-03-29 16:10     ` Daniel Bristot de Oliveira
2016-03-29 17:13       ` Steven Rostedt
2016-03-29 19:12         ` Daniel Bristot de Oliveira
2016-03-29 19:25           ` Steven Rostedt
2016-03-31  5:19             ` Juri Lelli

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=56FABD4E.3080008@redhat.com \
    --to=bristot@redhat.com \
    --cc=acme@redhat.com \
    --cc=juri.lelli@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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).