This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list
- From: Steven Rostedt <rostedt at goodmis dot org>
- To: Masami Hiramatsu <mhiramat at redhat dot com>
- Cc: Frederic Weisbecker <fweisbec at gmail dot com>, Ingo Molnar <mingo at elte dot hu>, lkml <linux-kernel at vger dot kernel dot org>, systemtap <systemtap at sources dot redhat dot com>, DLE <dle-develop at lists dot sourceforge dot net>, Jim Keniston <jkenisto at us dot ibm dot com>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Andi Kleen <ak at linux dot intel dot com>, Christoph Hellwig <hch at infradead dot org>, "Frank Ch. Eigler" <fche at redhat dot com>, "H. Peter Anvin" <hpa at zytor dot com>, Jason Baron <jbaron at redhat dot com>, "K.Prasad" <prasad at linux dot vnet dot ibm dot com>, Lai Jiangshan <laijs at cn dot fujitsu dot com>, Li Zefan <lizf at cn dot fujitsu dot com>, Peter Zijlstra <peterz at infradead dot org>, Srikar Dronamraju <srikar at linux dot vnet dot ibm dot com>, Tom Zanussi <tzanussi at gmail dot com>
- Date: Wed, 16 Sep 2009 10:29:31 -0400
- Subject: Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list
- References: <20090914204847.18779.69409.stgit@dhcp-100-2-132.bos.redhat.com> <20090914204904.18779.59422.stgit@dhcp-100-2-132.bos.redhat.com> <1253058681.20020.147.camel@gandalf.stny.rr.com> <4AB0F366.6080000@redhat.com>
- Reply-to: rostedt at goodmis dot org
On Wed, 2009-09-16 at 10:17 -0400, Masami Hiramatsu wrote:
> Steven Rostedt wrote:
> >> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> >> index ba34920..38e82a5 100644
> >> --- a/kernel/trace/trace_events.c
> >> +++ b/kernel/trace/trace_events.c
> >> @@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
> >> if (!d_events)
> >> return -ENOENT;
> >>
> >> + INIT_LIST_HEAD(&call->list);
> >
> > The INIT_LIST_HEAD is not needed here. The list_add will assign it.
>
> Without initializing it, list debugging code warns always :-)
> Please see, __list_add()@lib/list_debug.c
/me looks
from: include/linux/list.h
static inline void list_add(struct list_head *new, struct list_head *head)
{
__list_add(new, head, head->next);
}
from: lib/list_debug.c
void __list_add(struct list_head *new,
struct list_head *prev,
struct list_head *next)
{
WARN(next->prev != prev,
"list_add corruption. next->prev should be "
"prev (%p), but was %p. (next=%p).\n",
prev, next->prev, next);
WARN(prev->next != next,
"list_add corruption. prev->next should be "
"next (%p), but was %p. (prev=%p).\n",
next, prev->next, prev);
next->prev = new;
new->next = next;
new->prev = prev;
prev->next = new;
}
What you pass in is:
list_add(&call->list, &ftrace_events);
new = &call->list;
prev = &ftrace_events->prev;
next = &ftrace_events->next;
The above code never tests "new". The INIT_LIST_HEAD is useless.
-- Steve