All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Juri Lelli <juri.lelli@gmail.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 3/4] sched/deadline: Tracepoints for deadline scheduler
Date: Mon, 22 Feb 2016 22:30:17 +0100	[thread overview]
Message-ID: <20160222213017.GN6357@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20160222124854.3816fec4@gandalf.local.home>

On Mon, Feb 22, 2016 at 12:48:54PM -0500, Steven Rostedt wrote:

> > As it stands, the existing tracepoint have already been an ABI
> > trainwreck, why would I want to add more?
> 
> Yes, this may become a type of ABI, but even the sched switch
> tracepoints haven't been that bad. Has it really prevented us from
> changing anything?

The whole wakeup thing where we _still_ have a dummy argument, and have
been lying about the value for a long time really stinks.

> But let me ask, what would you recommend to finding out if the kernel
> has really given your tasks the recommended runtime within a given
> period? We can't expect users of SCHED_DEADLINE to be modifying the
> kernel themselves.

So why are these deadline specific tracepoint? Why not extend the ones
we already have?

Also, will these tracepoints still work if we implement SCHED_DEADLINE
using Least-Laxity-First or Pfair or some other exotic algorithm? Or
will be forever be bound to EDF just because tracepoint ABI shite?

Worse, the proposed tracepoints are atrocious, look at crap like this:

> +		if (trace_sched_deadline_yield_enabled()) {
> +			u64 delta_exec = rq_clock_task(rq) - p->se.exec_start;
> +			/* Subtract the last run till now */
> +			if (likely((s64)delta_exec > 0))
> +				p->dl.runtime -= delta_exec;
> +			trace_sched_deadline_yield(&p->dl);
> +		}

tracepoints should _NEVER_ change state, ever.

And there's the whole COND tracepoint muck, which also doesn't win any
prices.

So tell me why these specific tracepoints and why the existing ones
could not be extended to include this information. For example, why a
trace_sched_dealine_yield, and not a generic trace_sched_yield() that
works for all classes.

Tell me that the information presented does not pin the implementation.

And clean up the crap.

Then I might maybe consider this.

But do not present me with a bunch of random arse, hacked together
tracepoints and tell me they might be useful, maybe.

  parent reply	other threads:[~2016-02-22 21:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22 17:08 [PATCH 0/4] Tracepoints for deadline scheduler Daniel Bristot de Oliveira
2016-02-22 17:08 ` [PATCH 1/4] tracing: Add __print_ns_to_secs() and __print_ns_without_secs() helpers Daniel Bristot de Oliveira
2016-02-22 17:08 ` [PATCH 2/4] sched: Move deadline container_of() helper functions into sched.h Daniel Bristot de Oliveira
2016-02-22 17:08 ` [PATCH 3/4] sched/deadline: Tracepoints for deadline scheduler Daniel Bristot de Oliveira
2016-02-22 17:32   ` Peter Zijlstra
2016-02-22 17:48     ` Steven Rostedt
2016-02-22 20:11       ` Daniel Bristot de Oliveira
2016-02-22 21:30       ` Peter Zijlstra [this message]
2016-02-22 22:30         ` Steven Rostedt
2016-02-23 10:40           ` Juri Lelli
2016-02-23 10:44           ` Peter Zijlstra
2016-02-23 13:10             ` Steven Rostedt
2016-02-24  8:48               ` Ingo Molnar
2016-02-23 14:27             ` Peter Zijlstra
2016-02-23 16:19             ` Daniel Bristot de Oliveira
2016-02-24  2:29             ` Daniel Bristot de Oliveira
2016-02-22 17:48   ` kbuild test robot
2016-02-22 17:08 ` [PATCH 4/4] tools lib traceevent: Implements '%' operation Daniel Bristot de Oliveira
2016-02-22 20:23   ` Steven Rostedt
2016-02-23 14:38     ` Arnaldo Carvalho de Melo
2016-02-23 14:44       ` Arnaldo Carvalho de Melo
2016-02-25  5:41   ` [tip:perf/core] tools lib traceevent: Implement " tip-bot for Daniel Bristot de Oliveira

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=20160222213017.GN6357@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@redhat.com \
    --cc=bristot@redhat.com \
    --cc=juri.lelli@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.