From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: David Miller <davem@davemloft.net>
Cc: rostedt@goodmis.org, richm@oldelvet.org.uk,
609371@bugs.debian.org, ben@decadent.org.uk,
sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org,
fweisbec@gmail.com, mingo@redhat.com
Subject: Re: Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
Date: Wed, 19 Jan 2011 10:33:26 -0500 [thread overview]
Message-ID: <20110119153326.GC11022@Krystal> (raw)
In-Reply-To: <20110118.232045.58440904.davem@davemloft.net>
* David Miller (davem@davemloft.net) wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 18 Jan 2011 22:32:47 -0800 (PST)
>
> > As far as GCC can see, the object is static and also not part of an
> > array or any other C construct for which things like this could matter
> > as long as the alignment it chooses meets the minimum alignment
> > requirements of the ABI.
> >
> > So it doesn't let us do this trick where we put the individual event
> > markers into a special section, yet mark it __used and static, then
> > later access them as if they were part of a globally visible array.
> >
> > If you look these static objects, they are emitted with a leading
> > ".align 32" directive. This is what screws everything up.
> >
> > When the linker sees this, it aligns the start of every individual
> > "_ftrace_events" section, and that's where the "gaps" come from and
> > the crashes.
>
> I've come up with one possible way to deal with this.
>
> Put pointers to the trace objects into the special section, and
> interate over those instead.
>
> I was wondering why this x86-64 weird alignment behavior doesn't bite
> us for our init funcs. And the reason is that all of these weird
> alignment cases only trigger for aggregates (ie. structs and unions).
>
> So we could do:
>
> static struct ftace_event_call foo_event = { ... };
> static struct ftrace_event_call * __used
> __attribute__((section("_ftrace_event_ptrs")))
> foo_event_ptr = &foo_event;
>
> and
>
> extern struct ftrace_event_call *__start_ftrace_event_ptrs[];
> extern struct ftrace_event_call *__end_ftrace_event_ptrs[];
>
> struct ftrace_event_call **p = __start_ftrace_event_ptrs;
> while (p < &__end_ftrace_event_ptrs[0]) {
> struct ftrace_event_call *event = *p++;
>
> __trace_add_event_call(event, ...);
> }
>
> you get the idea.
>
> And we could mark this entire point section as "initdata" and thus
> free'able after bootup and post module load.
There are three downsides to this approach compared to forcing both the type and
variable alignments with attributes:
1) It consumes extra space: This approach lets gcc align foo_event on 32-byte
boundaries, which is unneeded in this case. For structures repeated very
often, this is a bad thing.
2) It mixes .data and struct ftrace_event_call definitions, thus polluting .data
cache-lines (actually, the 32-byte alignment will leave some of these
cachelines partly unused). This would be fixable by specifying yet another
section name to hold the struct ftace_event_call definitions.
3) Freeing _ftrace_event_ptrs is only possible after init here because Ftrace
data layout is sub-optimal. The linked-list created at init time by Ftrace
kind of duplicates the arrays already implicit to the section. If we look at
Tracepoints for example, which present the exact same section alignment
problem, we iterate on the tracepoint section each time a tracepoint is
activated or deactivated. So we need to keep the section there after init.
Therefore, your indirection approach would grow the data footprint.
The trade-off here is that walking the table is a very rare operation that
does not need to be fast, so we accept the performance hit of walking the
tracepoint table for each enabled tracepoint to shrink the memory footprint.
Especially for reasons (1) and (2), I'd be tempted to go for the
__long_long_aligned type and variable attribute instead. Thoughts ?
I'm still unsure that __long_long_aligned is needed over __long_aligned though.
AFAIK, the only requirement we have for, e.g. tracepoints, is to align on the
pointer size (sizeof(long)), so RCU pointer updates are performed atomically.
Aligning on the pointer size also allows the architecture to efficiently read
the field content. What does aligning on sizeof(long long) bring to us ? Is it
that you are concerned about the fact that the "aligned" type attribute, when
applied to a structure, is only used as a lower-bound by the compiler ? In that
case, we might want to consider using "packed" too:
#define __long_packed_aligned __attribute__((__packed__, __aligned__(__alignof__(long))))
I would just like to know if this attribute causes gcc to emit slower memory
access instructions on ppc/sparc/mips (I remember that at least one of these
emit slower instructions for unaligned read/writes, so I wonder if the compiler
uses them as soon as it sees the "packed" attribute, or if it is more clever
than that).
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
next prev parent reply other threads:[~2011-01-19 15:33 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20110113.155700.102679408.davem@davemloft.net>
[not found] ` <4D302B2F.7030108@oldelvet.org.uk>
[not found] ` <4D3074FE.3030707@oldelvet.org.uk>
2011-01-16 5:17 ` Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36 David Miller
2011-01-16 14:17 ` Richard Mortimer
2011-01-16 19:39 ` David Miller
2011-01-17 14:11 ` Steven Rostedt
2011-01-17 14:37 ` Bastian Blank
2011-01-17 19:35 ` Mathieu Desnoyers
2011-01-18 6:36 ` David Miller
2011-01-18 5:34 ` David Miller
2011-01-18 6:00 ` David Miller
2011-01-18 6:08 ` David Miller
2011-01-18 16:46 ` Mathieu Desnoyers
2011-01-18 17:33 ` Steven Rostedt
2011-01-18 18:16 ` Steven Rostedt
2011-01-18 18:26 ` Steven Rostedt
2011-01-18 20:13 ` Mathieu Desnoyers
2011-01-18 20:22 ` Steven Rostedt
2011-01-19 5:08 ` Mathieu Desnoyers
2011-01-19 5:16 ` David Miller
2011-01-19 15:10 ` Mathieu Desnoyers
2011-01-19 16:14 ` Sam Ravnborg
2011-01-19 16:18 ` Mathieu Desnoyers
2011-01-19 6:32 ` David Miller
2011-01-19 7:20 ` David Miller
2011-01-19 15:33 ` Mathieu Desnoyers [this message]
2011-01-19 21:40 ` David Miller
2011-01-19 22:00 ` Steven Rostedt
2011-01-19 22:09 ` David Miller
2011-01-19 22:21 ` Mathieu Desnoyers
2011-01-19 22:23 ` David Miller
2011-01-19 22:32 ` Sam Ravnborg
2011-01-19 22:34 ` Mathieu Desnoyers
2011-01-19 22:13 ` Mathieu Desnoyers
2011-01-19 22:21 ` David Miller
2011-01-19 22:33 ` Mathieu Desnoyers
2011-01-20 0:41 ` David Miller
2011-01-21 0:04 ` Mathieu Desnoyers
2011-01-21 18:06 ` Richard Mortimer
2011-01-21 18:52 ` Mathieu Desnoyers
2011-01-21 19:15 ` Mathieu Desnoyers
2011-01-21 20:14 ` Richard Mortimer
2011-01-21 20:40 ` Mathieu Desnoyers
2011-01-21 22:50 ` Richard Mortimer
2011-01-22 18:42 ` Richard Mortimer
2011-01-22 18:53 ` Mathieu Desnoyers
2011-01-19 15:46 ` Steven Rostedt
2011-01-19 16:15 ` Mathieu Desnoyers
2011-01-19 18:13 ` Steven Rostedt
2011-01-19 18:20 ` Mathieu Desnoyers
2011-01-19 21:44 ` David Miller
2011-01-19 22:15 ` Mathieu Desnoyers
2011-01-19 22:22 ` David Miller
2011-01-19 15:11 ` Mathieu Desnoyers
2011-01-19 15:27 ` Richard Mortimer
2011-01-17 6:07 ` David Miller
2011-01-17 9:05 ` Jesper Nilsson
2011-02-01 5:11 ` David Miller
2011-02-01 10:03 ` Jesper Nilsson
2011-01-17 10:22 ` Richard Mortimer
2011-01-17 14:15 ` Steven Rostedt
2011-01-18 6:35 ` David Miller
2011-01-18 17:30 ` Steven Rostedt
2011-01-17 19:46 ` R_SPARC_13 (Re: Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36) Richard Mortimer
2011-01-17 21:02 ` R_SPARC_13 David Miller
2011-01-17 23:34 ` R_SPARC_13 Richard Mortimer
2011-01-18 0:18 ` R_SPARC_13 David Miller
2011-01-18 0:37 ` R_SPARC_13 David Miller
2011-01-18 1:28 ` R_SPARC_13 Richard Mortimer
2011-01-18 6:50 ` R_SPARC_13 David Miller
2011-01-18 10:52 ` R_SPARC_13 Richard Mortimer
2011-01-18 13:23 ` R_SPARC_13 Richard Mortimer
2011-01-18 21:00 ` R_SPARC_13 David Miller
2011-01-19 4:12 ` R_SPARC_13 David Miller
2011-01-17 14:39 ` Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36 Bernhard R. Link
2011-01-18 5:24 ` David Miller
2011-01-18 9:26 ` Jesper Nilsson
2011-01-18 6:27 ` David Miller
2011-01-18 17:05 ` 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=20110119153326.GC11022@Krystal \
--to=mathieu.desnoyers@efficios.com \
--cc=609371@bugs.debian.org \
--cc=ben@decadent.org.uk \
--cc=davem@davemloft.net \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=richm@oldelvet.org.uk \
--cc=rostedt@goodmis.org \
--cc=sparclinux@vger.kernel.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