From: Johannes Berg <johannes@sipsolutions.net>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Andrew Morton <akpm@linux-foundation.org>,
Wolfgang Grandegger <wg@grandegger.com>
Subject: Re: [RFC - beta][PATCH] tracing: Introduce TRACE_MARKER() no argument trace event
Date: Tue, 11 Feb 2014 13:24:02 +0100 [thread overview]
Message-ID: <1392121442.4128.14.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <20140207152814.16e263ab@gandalf.local.home>
On Fri, 2014-02-07 at 15:28 -0500, Steven Rostedt wrote:
> I'm thinking no. And unless someone can give me a good reason that we
> should have such a thing, I will Nack my own patch!
:)
> What's up with trace_iwlwifi_dev_irq()? You are tracing it but not
> saving any information. There's obviously a reason for the
> iwl_pcie_isr() to be called (it's handling something). Does it do the
> exact same thing every time? No variables are needed then?
>
> What I'm trying to say is, if you go through the trouble of inserting a
> tracepoint into some location, might as well extract data from it.
> Otherwise you are wasting space in memory. If all you want to know is
> if the function was called or not, then simply use a kprobe or function
> trace that function (noinline to allow ftrace to trace it).
I think right now the tracepoint is misplaced, I meant it to be in the
function that's now iwl_pcie_isr() but I guess I haven't used it much.
In any case, the reasoning was that we don't yet know any useful data,
we really only know that an interrupt happened. We later have this ICT
(interrupt cause table or something) that we read the information from,
but for tracing I want to know each field in the table even though only
the OR of all of them is later relevant.
So originally we had "interrupt" and then "interrupt reason"
tracepoints. Looking at the code now, that seems to have been lost, so I
guess I should instead add the reason to the tracepoint and only have
the ICT tracing separate when ICT is enabled (in fact, if ICT isn't
enabled, I don't have the reason status at all...)
Looking at this I'd agree that having a special 'hit only' trace event
isn't really needed (any more), at least not for this particular case.
johannes
next prev parent reply other threads:[~2014-02-11 12:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-07 16:23 [RFC - beta][PATCH] tracing: Introduce TRACE_MARKER() no argument trace event Steven Rostedt
2014-02-07 20:28 ` Steven Rostedt
2014-02-11 12:24 ` Johannes Berg [this message]
2014-02-13 14:26 ` Frederic Weisbecker
2014-02-13 15:25 ` Steven Rostedt
2014-02-13 18:42 ` Frederic Weisbecker
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=1392121442.4128.14.camel@jlt4.sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=wg@grandegger.com \
/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