From: Steven Rostedt <rostedt@goodmis.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Pekka Paalanen <pq@iki.fi>,
linux-kernel@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Mathieu Desnoyers <compudj@krystal.dyndns.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Steven Rostedt <srostedt@redhat.com>
Subject: Re: [PATCH 6/6] ftrace: take advantage of variable length entries
Date: Wed, 1 Oct 2008 10:52:06 -0400 (EDT) [thread overview]
Message-ID: <alpine.DEB.1.10.0810011049030.30777@gandalf.stny.rr.com> (raw)
In-Reply-To: <20081001075419.GA24509@elte.hu>
On Wed, 1 Oct 2008, Ingo Molnar wrote:
>
> * Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > > > struct trace_entry *entry = iter->ent;
> > > > - struct mmiotrace_map *m = &entry->field.mmiomap;
> > > > + struct mmiotrace_map *m = (struct mmiotrace_map *)entry;
> > >
> > > This is different style than above, missing the struct
> > > trace_mmiotrace_map intermediate step. Looks like a bug,
> > > since struct mmiotrace_map is not the first field in
> > > struct trace_mmiotrace_map.
> >
> > Crap! yes this is a bug. Thanks for pointing this out.
>
> hm, there's a significant amount of type casts, see the grep below.
Yeah, I don't like that either.
>
> The ringbuffer becoming type-opaque has exactly these kinds of dangers,
> and as i suggested a few days ago, please think about a debug mode that
> stores the record type in the trace entry and validates it on extraction
> or something like that.
>
> That might even be a robustness feature: the tracer should not crash if
> the trace buffer gets corrupted. ftrace had that invariant before, i
> think we should try to keep as many aspects of it as possible.
Although the ring buffer is type opaque, that isn't the cause of this
issue. Ftrace still adds the types as needed. But since we no longer have
the big "one type for all" union entry, we need to be a bit more careful.
All types start with a one byte type id (giving us 255 types to use for
ftrace, zero being saved).
The following patch fixes this bug and addresses the issue.
-- Steve
next prev parent reply other threads:[~2008-10-01 14:52 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-30 3:02 [PATCH 0/6] ftrace: port to the new ring_buffer Steven Rostedt
2008-09-30 3:02 ` [PATCH 1/6] ftrace: give time for wakeup test to run Steven Rostedt
2008-09-30 3:02 ` [PATCH 2/6] Unified trace buffer Steven Rostedt
2008-09-30 9:45 ` Ingo Molnar
2008-09-30 11:53 ` Ingo Molnar
2008-09-30 12:06 ` Steven Rostedt
2008-09-30 3:02 ` [PATCH 3/6] ring_buffer: add paranoid check for buffer page Steven Rostedt
2008-09-30 3:02 ` [PATCH 4/6] ring_buffer: reset buffer page when freeing Steven Rostedt
2008-09-30 3:02 ` [PATCH 5/6] ftrace: make work with new ring buffer Steven Rostedt
2008-09-30 3:02 ` [PATCH 6/6] ftrace: take advantage of variable length entries Steven Rostedt
2008-09-30 17:33 ` Pekka Paalanen
2008-09-30 17:45 ` Steven Rostedt
2008-10-01 4:42 ` Steven Rostedt
2008-10-01 7:54 ` Ingo Molnar
2008-10-01 14:52 ` Steven Rostedt [this message]
2008-10-01 14:52 ` [PATCH] ftrace: type cast verifier Steven Rostedt
2008-10-01 17:42 ` Ingo Molnar
2008-10-01 17:47 ` Ingo Molnar
2008-09-30 7:45 ` [PATCH 0/6] ftrace: port to the new ring_buffer Ingo Molnar
2008-09-30 8:23 ` Ingo Molnar
2008-09-30 8:30 ` Ingo Molnar
2008-09-30 9:13 ` Ingo Molnar
2008-09-30 12:04 ` Steven Rostedt
2008-09-30 12:04 ` Steven Rostedt
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=alpine.DEB.1.10.0810011049030.30777@gandalf.stny.rr.com \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=compudj@krystal.dyndns.org \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=pq@iki.fi \
--cc=srostedt@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/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).