[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4ACBC510.1060006@gmail.com>
Date: Tue, 06 Oct 2009 15:30:40 -0700
From: "Justin P. Mattock" <justinmattock@...il.com>
To: Jason Baron <jbaron@...hat.com>
CC: Steven Rostedt <rostedt@...dmis.org>, Ingo Molnar <mingo@...e.hu>,
Peter Zijlstra <peterz@...radead.org>,
Li Zefan <lizf@...fujitsu.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: system gets stuck in a lock during boot
Jason Baron wrote:
> On Mon, Oct 05, 2009 at 09:24:09PM -0400, Steven Rostedt wrote:
>
>> On Fri, 2009-10-02 at 17:12 -0400, Jason Baron wrote:
>>
>>
>>> hi Justin,
>>>
>>> I've been playing around with gcc '4.5' as well and hit a panic that
>>> looks very similar to what you've seen with stock 2.6.31 - I haven't
>>> seen it anywhere else. Anyways, it seems to be some sort of alignment
>>> issue with the 'struct ftrace_event_call'. I'm not sure yet if this is a
>>> compiler or kernel issue. But the following kernel patch fixes the issue
>>> for me. It would be interesting to verify if the patch also resolves the
>>> issue for you.
>>>
>>> thanks,
>>>
>>> -Jason
>>>
>>>
>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>>> index 6ad76bf..0029af4 100644
>>> --- a/include/asm-generic/vmlinux.lds.h
>>> +++ b/include/asm-generic/vmlinux.lds.h
>>> @@ -164,6 +164,7 @@
>>> LIKELY_PROFILE() \
>>> BRANCH_PROFILE() \
>>> TRACE_PRINTKS() \
>>> + . = ALIGN(32); \
>>> FTRACE_EVENTS() \
>>> TRACE_SYSCALLS()
>>>
>>> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
>>> index a81170d..43f9f1e 100644
>>> --- a/include/linux/ftrace_event.h
>>> +++ b/include/linux/ftrace_event.h
>>> @@ -124,7 +124,7 @@ struct ftrace_event_call {
>>> atomic_t profile_count;
>>> int (*profile_enable)(struct ftrace_event_call *);
>>> void (*profile_disable)(struct ftrace_event_call *);
>>> -};
>>> +} __attribute__((aligned(32)));
>>>
>>> #define MAX_FILTER_PRED 32
>>> #define MAX_FILTER_STR_VAL 128
>>> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
>>> index f64fbaa..4697fb6 100644
>>> --- a/include/trace/ftrace.h
>>> +++ b/include/trace/ftrace.h
>>> @@ -600,7 +600,7 @@ static int ftrace_raw_init_event_##call(void) \
>>> } \
>>> \
>>> static struct ftrace_event_call __used \
>>> -__attribute__((__aligned__(4))) \
>>> +__attribute__((__aligned__(32))) \
>>> __attribute__((section("_ftrace_events"))) event_##call = { \
>>> .name = #call, \
>>> .system = __stringify(TRACE_SYSTEM), \
>>>
>> Are all alignments needed? Or just adding one might help. Or removing
>> the one directly above?
>>
>> -- Steve
>>
>>
>
> So the problem I'm seeing is an oops on boot caused by the call->system pointer
> deference in event_create_dir(). The 'call' variable is of type 'struct
> ftrace_event_call'.
>
> What's going on is that the 'struct ftrace_event_call' is of size 168 bytes
> (sizeof(struct ftrace_event_call)) = 168 = 0xA8. However, in memory the
> structures are 16-byte aligned. Thus, the stride for walking through the
> pointers needs to be 176 (0xB0), but instead its 168 causing the oops.
>
> I've only seen this issue while using gcc (GCC) 4.5.0 20090916, on a
> vanilla 2.6.31 kernel.
>
> That said, I'm not sure the compiler is doing the wrong thing here. The
> 'struct ftrace_event_call' contains an embedded 'struct list_head' which
> is 16 bytes. According to the gcc docs, the aligned attribute, 'specifies a
> minimum alignment for the variable or structure field, measured in bytes'.
> Thus, at least according to the docs, gcc can increase the alignment of the
> 'struct ftrace_event_call', from its original specification of 4, to 16. Even
> in the case where we are working corectly the structures are 8-byte aligned.
>
> Thus, I would reccommend the patch below as a preventive measure. Its
> the minimal patch I've found to resolve this issue. In general, if we
> are going to walk data structures embedded in a special elf section, I
> think the general rules needs to be to set the alignment to the power of
> two which is greater than or equal to the largest item in the structure.
>
> thanks,
>
> -Jason
>
> Signed-off-by: Jason Baron<jbaron@...hat.com>
>
>
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index a81170d..7182f03 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -124,7 +124,10 @@ struct ftrace_event_call {
> atomic_t profile_count;
> int (*profile_enable)(struct ftrace_event_call *);
> void (*profile_disable)(struct ftrace_event_call *);
> -};
> +} __attribute__((aligned(16)));
> +
> +/* Align to the largest field in the data structure:
> + * sizeof(struct list_head) = 16 */
>
> #define MAX_FILTER_PRED 32
> #define MAX_FILTER_STR_VAL 128
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index f64fbaa..e344e81 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -600,7 +600,6 @@ static int ftrace_raw_init_event_##call(void) \
> } \
> \
> static struct ftrace_event_call __used \
> -__attribute__((__aligned__(4))) \
> __attribute__((section("_ftrace_events"))) event_##call = { \
> .name = #call, \
> .system = __stringify(TRACE_SYSTEM), \
>
>
>
>
>
shoot I don't know why this is still hitting.
tried both patches and still.
As of now the only thing I can think of besides looking
at kernel/compiler is the patch for sysvinit to load
the policy(maybe something in there is old/outdated).
(BTW: not sure if it means anything but this system is x86_64
built from the multilib clfs, but with no 32 bit libs, pretty much
how fedora11 has there system built)
Justin P. Mattock
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists