[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4ACC001D.4050105@gmail.com>
Date: Tue, 06 Oct 2009 19:42:37 -0700
From: "Justin P. Mattock" <justinmattock@...il.com>
To: rostedt@...dmis.org
CC: Jason Baron <jbaron@...hat.com>, 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
Steven Rostedt wrote:
> On Tue, 2009-10-06 at 16:32 -0400, Jason Baron wrote:
>
>
>> 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 */
>>
>
> Is this true for i386?
>
> I just tried this patch and it seems to work. Can you give it a try.
>
> Signed-off-by: Steven Rostedt<rostedt@...dmis.org>
>
>
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 4ec5e67..044b70d 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -133,7 +133,7 @@ struct ftrace_event_call {
> atomic_t profile_count;
> int (*profile_enable)(void);
> void (*profile_disable)(void);
> -};
> +} __attribute__((aligned(sizeof(struct list_head))));
>
> #define FTRACE_MAX_PROFILE_SIZE 2048
>
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index cc0d966..31e7637 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -501,7 +501,6 @@ static void ftrace_profile_disable_##call(void) \
> * }
> *
> * static struct ftrace_event_call __used
> - * __attribute__((__aligned__(4)))
> * __attribute__((section("_ftrace_events"))) event_<call> = {
> * .name = "<call>",
> * .system = "<system>",
> @@ -619,7 +618,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), \
>
>
>
>
o.k. applied your patch, but unfortunantly
I still am hitting this kernel panic.
must admit I have no idea why this is doing this.
(but am willing to sit through this, because eventually
sooner or later will hit this if I update gcc).
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