Bug#783259: Debian kernel API for 3.16.7
Hi Ben, Ben Hutchings b...@decadent.org.uk wrote: On Fri, 2015-04-24 at 15:28 -0400, Jon Bernard wrote: [...] This is fine, if we could have a way to distinguish between upstream 3.16.7 and our version. It appears that Ubuntu kernels introduce a UTS_UBUNTU_RELEASE_ABI into their kernels, which can be used to follow their own kernel versions. Could we perhaps do the same? I used to maintain OOT modules myself, trying to support basically every distribution as well as mainline and stable kernels. Version tests just don't scale to this. Instead, you need to use some kind of a configure script to check the API of your target kernel. Using version tests, we have scaled LTTng modules to support every kernels released over the past 5 years, from 2.6.36 to 4.0, including mainline and stable releases, as well as distribution kernels for every major distribution out there. I understand that this approach has not scaled well for your use case, but it appears to work pretty well for us. Without the ability to do conditionals comparing numeric values both in Makefiles and within C, I am not sure how tell the two different kernels apart, and therefore am rather stuck. ABI changes are indicated by bumping the number after the dash in the kernel version string (3.16.0-3 to 3.16.0-4). But you actually need to know about the API, not the ABI. Not all changes to the API change the ABI, and vice versa. Good point, the ABI is not exactly what we need to track. This should perhaps be referred to as a publicly exposed API then. Still, it would greatly help us to have this version number available as a numeric value. Thanks, Mathieu Ben. -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/96441799.35924.1429908648206.javamail.zim...@efficios.com
Bug#691427: Reliably reproduce Lenovo 180GB SSD issues
Hi, We spent a couple of days cornering what appears to be an issue with the Intel 520 SSD drives in Lenovo x230 laptops. It was first showing up on a clean Debian installation, while installing a guest operating system into a VM. Looking around on forums, there appears to be some people having issues with database workloads too. So I decided to create a small user-space program to repoduce the problem. IMPORTANT: Before you try it, be ready for a system crash requiring a cold reboot.. It's available at: git://git.efficios.com/test-ssd.git direct link to .c file: https://git.efficios.com/?p=test-ssd.git;a=blob;f=test-ssd-write.c;hb=refs/heads/master This program simply performs random-access-writes of 4Kb into a single file. Executive summary of our findings (the details are in the test-ssd-write.c header in the git repo): - We reproduced this issue on 4 x230 machines (all our x230 have 180GB Intel drives, and they are all affected), - We took a SSD from one of the machines, moved it into an x200, and the problem still occurs, - The problem seems to occur independently of the filesystem (reproduced on ext3 and ext4), - Problem reproduced by test-ssd-write.c (git tree above): After less than 5 minutes of the heavy write workload, we get SATA errors and we need to cold reboot the machine to access the drive again. Example usage (don't forget to prepare for a computer freeze): ./test-ssd-write somefileondisk 209715200 1234 -z (see options by just running ./test-ssd-write) The problem occurs with drive model SSDSC2BW180A3L, with both firmwares LE1i and LF1i (those are Lenovo firmwares). We could reproduce the issue on 3.2 (Debian), 3.5 (Debian), 3.7.9 (Arch) distribution kernels. We could reproduce it with x230 BIOS G2ET90WW (2.50) 2012-20-12 and G2ET86WW (2.06) 2012-11-13, but since it can be reproduced on a x200 too, it does not appear to be a BIOS issue. We tried the program on a range of other SSD drives, one of those including the same SandForce 2281 controller (details within test-ssd-write.c header). So our current guess is that the Lenovo firmware on the SSD might be part of the problem, but it might be good if we could to confirm that Intel's firmwares work fine. Thoughts, ideas, hints about who to contact on this issue would be very much welcome, Thanks, Mathieu Relevant Lenovo threads: - http://forums.lenovo.com/t5/X-Series-ThinkPad-Laptops/x230-SATA-errors-with-180GB-Intel-520-SSD-under-heavy-write-load/m-p/1068147/highlight/false#M48401 - http://forums.lenovo.com/t5/T400-T500-and-newer-T-series/T430s-Intel-SSD-520-180GB-issue/m-p/1070549#M76964 -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20130325140221.GA2018@Krystal
Re: [PATCH] module: Enable dynamic debugging regardless of taint
* Rusty Russell (ru...@rustcorp.com.au) wrote: On Tue, 01 Nov 2011 03:59:33 +, Ben Hutchings b...@decadent.org.uk wrote: Dynamic debugging is currently disabled for tainted modules, except for TAINT_CRAP. This prevents use of dynamic debugging for out-of-tree modules now that they are also tainted. This condition was apparently intended to avoid a crash if a force- loaded module has an incompatible definition of dynamic debug structures. However, a administrator that forces us to load a module is claiming that it *is* compatible even though it fails our version checks. If they are mistaken, there are any number of ways the module could crash the system. Signed-off-by: Ben Hutchings b...@decadent.org.uk Thanks, applied, unless Mathieu objects... I'm OK with that. We should probably note in the changelog the side-effect of now supporting dynamic debugging of proprietary drivers. If we start doing this for dynamic debugging, I'd be tempted to do it for tracepoints and static jump labels too, since at least tracepoints would not be the only offender (from an end-user point of view) causing crashes on incompatible module load. Thanks! Acked-by: Mathieu Desnoyers mathieu.desnoy...@efficios.com Cheers, Rusty. -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/2001124859.GA1698@Krystal
Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree
* Rusty Russell (ru...@rustcorp.com.au) wrote: On Tue, 25 Oct 2011 16:17:24 -0400, Dave Jones da...@redhat.com wrote: commit 7816c45bf13255157c00fb8aca86cb64d825e878 Author: Roland Vossen rvos...@broadcom.com Date: Thu Apr 7 11:20:58 2011 +0200 modules: Enabled dynamic debugging for staging modules ... Signed-off-by: Roland Vossen rvos...@broadcom.com Acked-by: Jason Baron jba...@redhat.com Signed-off-by: Greg Kroah-Hartman gre...@suse.de Greg, you know better. This is why we have maintainers: I can't track patches I don't see. Grrr... If we want to support out of tree modules with this, should we just nuke the whole check, or do we still want to prevent certain types of tainted kernels from using this stuff ? It goes back to the first implementation of kernel markers. IIRC, it was to prevent dynamic debug stuff from circumventing licensing, but testing for *any* taint seems overbroad. Mathieu? This check for tainted modules was first introduced with markers, and then used by tracepoints, and then also by dynamic debug. The rationale for this check was mainly to ensure that the marker/tracepoint code would not trigger a crash when loading a module with incompatible module header, originally compiled for an older kernel, into a newer kernel. This problem would happen even if the said module does not contain any marker/tracepoint, because we happen to try to use fields that are non-existent in the module header. AFAIK, dynamic debug use a similar trick that require extra members in the module header, so checking that the module header format is compatible with the kernel would be enough. Is there a taint flag that allows us to check for this more narrowly ? TAINT_FORCED_MODULE would probably be the closest one we have now, although we might want to be more specific than that. Thanks, Mathieu Thanks, Rusty. PS. Can't see how this related to lockdep either... -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20111026061521.GB16408@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Richard Mortimer (ri...@oldelvet.org.uk) wrote: On 21/01/2011 22:50, Richard Mortimer wrote: On 21/01/2011 20:40, Mathieu Desnoyers wrote: * Richard Mortimer (ri...@oldelvet.org.uk) wrote: Thanks for the info! At first glance, it does not seem to contradict my findings. When you find time, can you have a try at v3 I just posted ? I'm setting the compilation off now. I will give it a whirl some time Saturday or Sunday. ... Make sure to start tracing in your tests, as I think the unaligned access only happens when accessing the struct tracepoint fields below the int state field. I have the revised patch running now with tracing enabled. Everything looks good to me. Thanks for testing it! Mathieu Thanks. Richard -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110122185326.GA3969@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Richard Mortimer (ri...@oldelvet.org.uk) wrote: On 21/01/2011 00:04, Mathieu Desnoyers wrote: * David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyersmathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 17:33:39 -0500 So I guess we go for the following. Is it verbose enough ? It's got all of the details that seem to matter, thanks. I'm letting people following this thread and the bug report know that I posted the patch we discussed about here in the following thread: https://lkml.org/lkml/2011/1/19/509 Feedback is welcome, Hi Richard, Hi Mathieu, Thanks for this. The good news is that I have a 2.6.37 kernel + your patches booting and it has been running for about an hour on sparc64/Sun Fire V120. I did a bit of poking around in /debug and turned on/off a bit of tracing and nothing broke. That's a good start :) I did have a few problems applying your patch series. Your diffs have semicolons at the end of a couple of the macros that don't appear in the mainline 2.6.37. One was circa line 174 of include/linux/tracepoint.h but I don't have the other to hand at the moment and I don't have much time before I need to go out. I know what's causing this, I'll rebase my patch before my next post (I was working at the tail of my lttng patch queue). Thanks for letting me know. I'm also getting a lot of Kernel unaligned access errors from the kernel. I don't know if they are related to this or not and this is the first time that I personally have got 2.6.37 to boot on sparc64. The messages that I am getting seem to be repeats of [ 4376.807811] Kernel unaligned access at TPC[456e94] try_to_wake_up+0x58/0xec [ 4376.807908] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660 [ 4376.808044] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660 [ 4376.808871] Kernel unaligned access at TPC[456e94] try_to_wake_up+0x58/0xec [ 4376.808965] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660 [ 4381.813354] log_unaligned: 337 callbacks suppressed I have to go out now but will be around later/over the weekend. Can you send me your .config ? I'd really like to see which of tracepoint/static jump patching are enabled. Do you get this message as soon as the system boots, or only when you enable tracing ? Thanks, Mathieu Regards Richard -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110121185259.GA13198@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Mathieu Desnoyers (mathieu.desnoy...@efficios.com) wrote:
* Richard Mortimer (ri...@oldelvet.org.uk) wrote:
[...]
I'm also getting a lot of Kernel unaligned access errors from the
kernel. I don't know if they are related to this or not and this is the
first time that I personally have got 2.6.37 to boot on sparc64. The
messages that I am getting seem to be repeats of
[ 4376.807811] Kernel unaligned access at TPC[456e94]
try_to_wake_up+0x58/0xec
[ 4376.807908] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660
[ 4376.808044] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660
[ 4376.808871] Kernel unaligned access at TPC[456e94]
try_to_wake_up+0x58/0xec
[ 4376.808965] Kernel unaligned access at TPC[75541c] schedule+0x454/0x660
[ 4381.813354] log_unaligned: 337 callbacks suppressed
I have to go out now but will be around later/over the weekend.
OK, I pinpointed it to my use of the packed attribute. So within the
structure:
struct test {
const char *a;
int b;
void *c;
void *d;
void *e;
} __attribute__((packed, aligned(8)));
(on sparc64)
It provides the following offsets:
0 8 12 20 28
which is clearly wrong in terms of inner alignment: it removes the padding
between b and c. I am really tempted to just remove the packed attribute from
there: our goal is really to make sure the 8-byte accesses are all aligned after
all. So theoretically gcc could decide to align all struct test arrays and
pointers on an alignment larger than 8 if we just specify the aligned(8) type
attribute (because the type attribute is just a minimum floor alignment value),
but the only reason I would see for gcc to align these on larger alignment would
be that the structures would contain a field that requires such largish
alignment (which I doubt we have in the kernel).
I'll prepare updated patches shortly.
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency RD Consultant
EfficiOS Inc.
http://www.efficios.com
--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110121191556.GB13198@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Richard Mortimer (ri...@oldelvet.org.uk) wrote: [...] P.S. I saw your followup mail so hopefully this matches what you have found! Thanks for the info! At first glance, it does not seem to contradict my findings. When you find time, can you have a try at v3 I just posted ? Make sure to start tracing in your tests, as I think the unaligned access only happens when accessing the struct tracepoint fields below the int state field. Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110121204033.GA14125@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 17:33:39 -0500 So I guess we go for the following. Is it verbose enough ? It's got all of the details that seem to matter, thanks. I'm letting people following this thread and the bug report know that I posted the patch we discussed about here in the following thread: https://lkml.org/lkml/2011/1/19/509 Feedback is welcome, Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110121000421.GA5014@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 00:08:45 -0500 The following works fine for me now. Comments are welcome. Thanks for doing this work Mathieu. - No aligned() type attribute nor variable attribute. I get a crash on x86_64 (NULL pointer exception when executing __trace_add_event_call, the 5th call). __alignof__(struct ftrace_event_call) is worth 8. This is really bizarre. Does it only happen on x86_64? Sadly, my ppc32 test machine is currently broken, so I could not check on other than x86 archs. I'm wondering if GCC does something bizarre like work with different default alignments based upon the section or something like that. If so, maybe adding the section attribute to the array definition will fix things? Well, I thought about it in my sleep, and it looks like gcc is within its rights to align these statically declared structures on a larger alignment: gcc has no clue that we're going to do tricks with the linker to access the structures as an array, so aligning on a larger alignment *should* be fine for the compiler, but we suffer because we're doing something non-standard. On 32-bit architectures, we really want a aligned(4), and on 64-bit architectures, aligned(8). Represent this by creating: #define __long_aligned __attribute__((__aligned__(__alignof__(long Do any of these datastructures have, or will have, u64 or long long types in them? If so, then we will need to use 8 unconditionally or __alignof__(long long). If my memory serves me correctly, I think long long is aligned on 4 bytes on ppc32, but on 8 bytes on x86_32 (yeah, that's weird). How about we create a #define __long_long_aligned __attribute__((__aligned__(__alignof__(long long ? Thanks, Mathieu I'll see if I can work out why using no align directive explodes on x86-64. -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119151004.GA11022@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 00:08:45 -0500 - No aligned() type attribute nor variable attribute. I get a crash on x86_64 (NULL pointer exception when executing __trace_add_event_call, the 5th call). __alignof__(struct ftrace_event_call) is worth 8. I think I figured it out. It's the static vs. non-static thing, or some other crazyness wrt. how x86-64 implements it's alignment rules. GCC on x86-64 uses a completely different policy for aligning local (ie. static) data objects vs. globally visible ones, for one thing. It also has different alignment policies for objects that are part of an array vs. those which are not. On both counts, we're lying to the compiler, so maybe it's sort of our fault. Yep, we're in strong agreement here. 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. Ah, yep, good way to identify the culprit. 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. OK (more to come in the following email response). Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119151139.GB11022@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote:
From: David Miller da...@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 RD Consultant
EfficiOS Inc.
http://www.efficios.com
--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110119153326.GC11022@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote:
After applying David's remove align patch, I got it to boot on x86_64
with the following two patches. I thought just adding the align to the
structure declaration would work, but it still failed on the syscall for
init_module. By removing the double declaration of event_exit_##sname,
removed this problem.
I'll test this on x86 32bit and PPC 64. If it works there, I'll push all
of them out for 38. Should these go to 37 stable too?
Please hold before adding these patches into git. They don't seem to address the
underlying problem correctly. See the latest exchanges between David Miller and
myself for more info.
We need to come up with something better than it boots as an explanation for
the fix.
Thanks,
Mathieu
-- Steve
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index cacc27a..0e3bce6 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -142,8 +142,6 @@ extern struct trace_event_functions
exit_syscall_print_funcs;
#define SYSCALL_TRACE_EXIT_EVENT(sname)
\
static struct syscall_metadata \
__attribute__((__aligned__(4))) __syscall_meta_##sname; \
- static struct ftrace_event_call \
- __attribute__((__aligned__(4))) event_exit_##sname; \
static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section(_ftrace_events)))\
Index: linux-trace.git/include/linux/ftrace_event.h
===
--- linux-trace.git.orig/include/linux/ftrace_event.h
+++ linux-trace.git/include/linux/ftrace_event.h
@@ -194,7 +194,7 @@ struct ftrace_event_call {
int perf_refcount;
struct hlist_head __percpu *perf_events;
#endif
-};
+} __attribute__((__aligned__(32)));
#define PERF_MAX_TRACE_SIZE 2048
--
Mathieu Desnoyers
Operating System Efficiency RD Consultant
EfficiOS Inc.
http://www.efficios.com
--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110119161534.GB15031@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Sam Ravnborg (s...@ravnborg.org) wrote: If my memory serves me correctly, I think long long is aligned on 4 bytes on ppc32, but on 8 bytes on x86_32 (yeah, that's weird). How about we create a #define __long_long_aligned __attribute__((__aligned__(__alignof__(long long #define __u64_aligned __attribute__((__aligned__(__alignof__(long long A bit shorter but maybe less obvious. Yep, that would make sense. I'm tempted to try creating #defined __u64_packed_aligned __attribute__((__packed__, __aligned__(__alignof__(long long in the hope that gcc sees this as a strict alignment requirement (including a max bound) rather than just a hint. From what I gather in my reading of http://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html The aligned attribute can only increase the alignment; but you can decrease it by specifying packed as well. See below. gcc seems to support having both specified. I think this would provide the kind of alignment guarantees we really need here: both specifying the minimum _and_ maximum alignment. Thoughts ? Mathieu Sam -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119161857.GC15031@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote: On Wed, 2011-01-19 at 11:15 -0500, Mathieu Desnoyers wrote: * Steven Rostedt (rost...@goodmis.org) wrote: After applying David's remove align patch, I got it to boot on x86_64 with the following two patches. I thought just adding the align to the structure declaration would work, but it still failed on the syscall for init_module. By removing the double declaration of event_exit_##sname, removed this problem. I'll test this on x86 32bit and PPC 64. If it works there, I'll push all of them out for 38. Should these go to 37 stable too? Please hold before adding these patches into git. They don't seem to address the underlying problem correctly. See the latest exchanges between David Miller and myself for more info. We need to come up with something better than it boots as an explanation for the fix. Yes, I agree that we should solve this issue correctly. But if there is a work around to the problem, we could implement that if the real solution is not in our grasp yet. A known working workaround (used in tracepoints for a few years) is to align the type declaration on 32 bytes. It wastes space, but works. With this solution, you should remove all the per-variable alignment attributes. Now what I'm discussing with David Miller is if creating a __long_packed_aligned and using it for *both* type and variable alignment would be more palatable (it also works, and is more compact). David proposed a solution with an array of pointers (extra indirection) which I don't really like for 3 reasons I exposed in my reply to him. So it's not that the solution is not in our grasp yet, it's more that we have to choose the right one. Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119182052.GB18970@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote:
From: Mathieu Desnoyers mathieu.desnoy...@efficios.com
Date: Wed, 19 Jan 2011 10:33:26 -0500
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:
My concern is that if there is ever a u64 or similarly long long
typed member in these tracing structures, it will not be aligned
sufficiently to avoid unaligned access traps on 32-bit systems.
Hrm, I'd like to see what kind of ill-conceived 32-bit architecture would
generate a unaligned access for a 32-bit aligned u64. Do you have examples in
mind ? By definition, the memory accesses should be at most 32-bit, no ? AFAIK,
gcc treats u64 as two distinct reads on all 32-bit architectures.
If your suggestion defines the lowest possible alignment and GCC will
do the right thing and up-align the structure if necessary, then
fine.
Well, I must admit that my assumption is that aligning on the long size should
be the only alignment required, both on 32-bit and 64-bit. But I'm curious to
see if there are indeed architectures that break this assumption.
Ideally, I'd like to avoid letting gcc up-align a structure, because it is then
hard to know for sure what the alignment value of the section should be (in the
linker script, we can safely choose 32, but it's more a safe choice than
anything else). Moreover, I'm not convinced that gcc will choose to up-align the
structure with the exact same alignment values for both the type declaration and
the variable definition (I'm deeply distrusting gcc to do the right thing here).
If you add packed it is going to screw everything up and we'll
essentially be back to square one.
On RISC like sparc64, packed causes even 16-bit words to be read and
written a byte at a time.
Never use packed under any circumstances unless absolutely
unavoidable.
gcc on my sparc64 box (32-bit userland) disagrees with you here ;) Using
gcc (Debian 4.3.3-14) 4.3.3, here is a demonstration that, indeed, packed
generates aweful code, but that packed, aligned(4 or 8) generates pretty
decent code:
compiling for sparc32:
struct test {
unsigned long a;
unsigned long b;
};
Storing to test a field in a main() that returns 0, with -O0:
000104f0 main:
104f0: 9d e3 bf 90 save %sp, -112, %sp
104f4: 03 00 00 81 sethi %hi(0x20400), %g1
104f8: 84 10 63 9c or %g1, 0x39c, %g2 ! 2079c blah
104fc: 82 10 20 2a mov 0x2a, %g1
10500: c2 20 80 00 st %g1, [ %g2 ]
10504: 82 10 20 00 clr %g1
10508: b0 10 00 01 mov %g1, %i0
1050c: 81 e8 00 00 restore
10510: 81 c3 e0 08 retl
10514: 01 00 00 00 nop
__attribute__((packed))
000104f0 main:
104f0: 9d e3 bf 90 save %sp, -112, %sp
104f4: 03 00 00 81 sethi %hi(0x20400), %g1
104f8: 84 10 63 dc or %g1, 0x3dc, %g2 ! 207dc blah
104fc: c2 08 80 00 ldub [ %g2 ], %g1
10500: 82 08 60 00 and %g1, 0, %g1
10504: c2 28 80 00 stb %g1, [ %g2 ]
10508: c2 08 a0 01 ldub [ %g2 + 1 ], %g1
1050c: 82 08 60 00 and %g1, 0, %g1
10510: c2 28 a0 01 stb %g1, [ %g2 + 1 ]
10514: c2 08 a0 02 ldub [ %g2 + 2 ], %g1
10518: 82 08 60 00 and %g1, 0, %g1
1051c: c2 28 a0 02 stb %g1, [ %g2 + 2 ]
10520: c2 08 a0 03 ldub [ %g2 + 3 ], %g1
10524: 82 08 60 00 and %g1, 0, %g1
10528: 82 10 60 2a or %g1, 0x2a, %g1
1052c: c2 28 a0 03 stb %g1, [ %g2 + 3 ]
10530: 82 10 20 00 clr %g1
10534: b0 10 00 01 mov %g1, %i0
10538: 81 e8 00 00 restore
1053c: 81 c3 e0 08 retl
10540: 01 00 00 00 nop
__attribute__((packed, aligned(4)))
000104f0 main:
104f0: 9d e3 bf 90 save %sp, -112, %sp
104f4: 03 00 00 81 sethi %hi(0x20400), %g1
104f8: 84 10 63 9c or %g1, 0x39c, %g2 ! 2079c blah
104fc: 82 10 20 2a mov 0x2a, %g1
10500: c2 20 80 00 st %g1, [ %g2 ]
10504: 82 10 20 00 clr %g1
10508: b0 10 00 01 mov %g1, %i0
1050c: 81 e8 00 00 restore
10510: 81 c3 e0 08 retl
10514: 01 00 00 00 nop
__attribute__((packed, aligned(8)))
000104f0 main:
104f0: 9d e3 bf 90 save
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 13:20:53 -0500 Now what I'm discussing with David Miller is if creating a __long_packed_aligned and using it for *both* type and variable alignment would be more palatable (it also works, and is more compact). As I mentioned in another reply, we should not be using packed. Packed has other implications, which makes it use byte-at-a-time accesses for all parts of a structure when you tag it with 'packed'. GCC doesn't try to be clever and see that actually such accesses are safe. Please see my explanation about the difference between packed and packed, aligned(4 or 8) in the other thread. I share your concern about ugly code generated by packed, but my tests with a sparc32 gcc show that using both packed and aligned attributes generate nice code. If plain __long_aligned works and, since you're tagging it to the structure definition, it only specifies a minimum-alignment, then I'm fine with using that to fix this. I'd prefer to add the packed to ensure that gcc never decides for some odd reason to use an alignment larger than the one we specify in the linker script. Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119221538.GB23544@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote: On Wed, 2011-01-19 at 13:40 -0800, David Miller wrote: My concern is that if there is ever a u64 or similarly long long typed member in these tracing structures, it will not be aligned sufficiently to avoid unaligned access traps on 32-bit systems. The structure that gets placed in this section is the ftrace_event_call. It consists only of pointers, a struct list_head, a couple of int, and a struct trace_event, which consists of: a struct hlist_node, a struct list_head, an int, and a pointer. None of these are more than long and I don't foresee them needing a long long type. I think assuming that a long is the largest item should due. We can add a comment next to these structures specifying this dependency, and hopefully it would be updated if we ever do include a long long in them. I still wonder how a 32-bit system can generate an unaligned access trap for an access to a 64-bit variable aligned on 32-bit, given that there is, by definition, no 64-bit memory accesses available on the architecture ? Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119222144.GC23544@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Date: Wed, 19 Jan 2011 17:13:27 -0500 Hrm, I'd like to see what kind of ill-conceived 32-bit architecture would generate a unaligned access for a 32-bit aligned u64. Do you have examples in mind ? By definition, the memory accesses should be at most 32-bit, no ? AFAIK, gcc treats u64 as two distinct reads on all 32-bit architectures. Sparc 32-bit has 64-bit loads and stores, GCC uses them because the ABI specifies that every structure is at least 8 byte aligned. Ah, that's the answer I was looking for, thanks! gcc on my sparc64 box (32-bit userland) disagrees with you here ;) Using gcc (Debian 4.3.3-14) 4.3.3, here is a demonstration that, indeed, packed generates aweful code, but that packed, aligned(4 or 8) generates pretty decent code: Amazing, if this works then do it. But please document this fully with comments and such :-) I will, I will! ;) So I guess we go for the following. Is it verbose enough ? /* * __u64_packed_aligned: * * Forces gcc to use the u64 type alignment, up-aligning or down-aligning the * target type if necessary. The memory accesses to the target structure are * efficient (does not require bytewise memory accesses) and the atomic pointer * update guarantees required by RCU are kept. u64 is considered as the largest * type that can generate a trap for unaligned accesses (u64 on sparc32 needs to * be aligned on 64-bit). * * Specifying both packed and aligned generates decent code (without the * bytewise memory accesses generated by simply using packed), and forces * gcc to down-align the structure alignment to the alignment of a u64 type. * * This alignment should be used for both structure definitions and declarations * (as *both* the type and variable attribute) when using the section * attribute to generate arrays of structures. */ #define __u64_packed_aligned \ __attribute__((__packed__, __aligned__(__alignof__(long long Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119223339.GD23544@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Sam Ravnborg (s...@ravnborg.org) wrote: I still wonder how a 32-bit system can generate an unaligned access trap for an access to a 64-bit variable aligned on 32-bit, given that there is, by definition, no 64-bit memory accesses available on the architecture ? From the SPARC V8 manual (this is the 32 bit version of SPARC): Load/Store Instructions ... Integer load and store instructions support byte (8-bit), halfword (16-bit), word (32-bit), and doubleword (64-bit) accesses. ... Alignment Restrictions Halfword accesses must be aligned on a 2-byte boundary, word accesses (which include instruction fetches) must be aligned on a 4-byte boundary, and doubleword accesses must be aligned on an 8-byte boundary. An improperly aligned address causes a load or store instruction to generate a mem_address_not_aligned trap. Ah! There is always an odd case ;) Thanks! Mathieu -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110119223440.GE23544@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* David Miller (da...@davemloft.net) wrote: From: David Miller da...@davemloft.net Date: Mon, 17 Jan 2011 22:00:39 -0800 (PST) ftrace: Remove unnecessary alignment tag from ftrace_event_call. It's completely unnecessary and causes problems on platforms where this tag down-aligns the structure's alignment. Signed-off-by: David S. Miller da...@davemloft.net ... Ok, unless we can explain why these alignments are needed at all, we should kill all of them: ftrace: linker script add missing struct align We should add the missing STRUCT_ALIGN(); in include/asm-generic/vmlinux.lds.h as a preliminary step to remove the ftrace bogus structure alignments. Moving all STRUCT_ALIGN() for FTRACE_EVENTS() and TRACE_SYSCALLS() into the definitions, so the alignment is only done if these infrastructures are configured in. Also align TRACE_PRINTKS on 8 bytes to make sure the beginning of the section is aligned on pointer size. Signed-off-by: Mathieu Desnoyers mathieu.desnoy...@efficios.com --- include/asm-generic/vmlinux.lds.h | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h === --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h @@ -107,7 +107,8 @@ #endif #ifdef CONFIG_TRACE_BRANCH_PROFILING -#define LIKELY_PROFILE() VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \ +#define LIKELY_PROFILE() STRUCT_ALIGN(); \ + VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \ *(_ftrace_annotated_branch) \ VMLINUX_SYMBOL(__stop_annotated_branch_profile) = .; #else @@ -115,7 +116,8 @@ #endif #ifdef CONFIG_PROFILE_ALL_BRANCHES -#define BRANCH_PROFILE() VMLINUX_SYMBOL(__start_branch_profile) = .; \ +#define BRANCH_PROFILE() STRUCT_ALIGN(); \ + VMLINUX_SYMBOL(__start_branch_profile) = .; \ *(_ftrace_branch) \ VMLINUX_SYMBOL(__stop_branch_profile) = .; #else @@ -123,7 +125,8 @@ #endif #ifdef CONFIG_EVENT_TRACING -#define FTRACE_EVENTS()VMLINUX_SYMBOL(__start_ftrace_events) = .; \ +#define FTRACE_EVENTS()STRUCT_ALIGN(); \ + VMLINUX_SYMBOL(__start_ftrace_events) = .; \ *(_ftrace_events) \ VMLINUX_SYMBOL(__stop_ftrace_events) = .; #else @@ -131,7 +134,8 @@ #endif #ifdef CONFIG_TRACING -#define TRACE_PRINTKS() VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .; \ +#define TRACE_PRINTKS() . = ALIGN(8);\ +VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .; \ *(__trace_printk_fmt) /* Trace_printk fmt' pointer */ \ VMLINUX_SYMBOL(__stop___trace_bprintk_fmt) = .; #else @@ -139,7 +143,8 @@ #endif #ifdef CONFIG_FTRACE_SYSCALLS -#define TRACE_SYSCALLS() VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \ +#define TRACE_SYSCALLS() STRUCT_ALIGN(); \ +VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \ *(__syscalls_metadata) \ VMLINUX_SYMBOL(__stop_syscalls_metadata) = .; #else @@ -169,11 +174,7 @@ LIKELY_PROFILE()\ BRANCH_PROFILE()\ TRACE_PRINTKS() \ - \ - STRUCT_ALIGN(); \ FTRACE_EVENTS() \ - \ - STRUCT_ALIGN(); \ TRACE_SYSCALLS() /* -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20110118164633.GA12872@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote:
On Tue, 2011-01-18 at 13:16 -0500, Steven Rostedt wrote:
On Tue, 2011-01-18 at 12:33 -0500, Steven Rostedt wrote:
On Tue, 2011-01-18 at 11:46 -0500, Mathieu Desnoyers wrote:
Also align TRACE_PRINTKS on 8 bytes to make sure the beginning of the
section is
aligned on pointer size.
If I can make it crash without the alignments and this fixes the issue,
I'll apply both patches.
After applying David's patch, I do indeed get a crash. I'll now apply
yours and see if it fixes the issue.
Your patch doesn't seem to fix it either. I'll investigate this further.
I think David's patch missed kernel/trace/trace_export.c
struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section(_ftrace_events))) event_##call = { \
and kernel/trace/trace.h:
#define FTRACE_ENTRY(call, struct_name, id, tstruct, print) \
extern struct ftrace_event_call \
__attribute__((__aligned__(4))) event_##call;
does it help if you remove these ?
Mathieu
-- Steve
--
Mathieu Desnoyers
Operating System Efficiency RD Consultant
EfficiOS Inc.
http://www.efficios.com
--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110118201323.GA14930@Krystal
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote:
On Tue, 2011-01-18 at 15:13 -0500, Mathieu Desnoyers wrote:
* Steven Rostedt (rost...@goodmis.org) wrote:
On Tue, 2011-01-18 at 13:16 -0500, Steven Rostedt wrote:
On Tue, 2011-01-18 at 12:33 -0500, Steven Rostedt wrote:
On Tue, 2011-01-18 at 11:46 -0500, Mathieu Desnoyers wrote:
Also align TRACE_PRINTKS on 8 bytes to make sure the beginning of
the section is
aligned on pointer size.
If I can make it crash without the alignments and this fixes the
issue,
I'll apply both patches.
After applying David's patch, I do indeed get a crash. I'll now apply
yours and see if it fixes the issue.
Your patch doesn't seem to fix it either. I'll investigate this further.
I think David's patch missed kernel/trace/trace_export.c
struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section(_ftrace_events))) event_##call = { \
and kernel/trace/trace.h:
#define FTRACE_ENTRY(call, struct_name, id, tstruct, print) \
extern struct ftrace_event_call \
__attribute__((__aligned__(4))) event_##call;
does it help if you remove these ?
I doubt it, but I'll try it anyway.
The following works fine for me now. Comments are welcome.
tracing: use __aligned__() variable and type attributes to work around gcc bug
I just played with the possible combinations, and here are my current results,
with the appropriately aligned linker script sections (with my patch applied):
- No aligned() type attribute nor variable attribute. I get a crash on x86_64
(NULL pointer exception when executing __trace_add_event_call, the 5th call).
__alignof__(struct ftrace_event_call) is worth 8.
- adding type attribute aligned(32) on the struct ftrace_event_call and
struct syscall_metadata declarations seems to work fine, but consumes space
needlessly (struct ftrace_event_call grows from 136 to 160 bytes). Note that
tracepoints use the type attribute aligned(32), which falls in this category
(waste of space). We might want to do better.
- adding type attribute aligned(8) crashes. Note that the gcc documentation for
type attributes explains that this specifies the minimum alignment for the
type, so the compiler is free to choose a larger one.
- adding type attribute (packed, aligned(8)) should force the compiler to
consider a 8-byte alignment for the type (as shown by __alignof__), but it
doesn't work (crash).
I really don't like specifying alignment as variable attributes (rather than
type attributes), because the extern array that iterates on the attributes has
no clue of the alignment used (same for pointer arithmetic). Unfortunately, it
seems to be the only way to really make sure gcc does not use an alignment
larger than prescribed.
So far, my somewhat premature conclusion is that gcc honours the aligned() type
attribute for the array iteration (thus expecting a 136 bytes array aligned on 8
bytes boundaries), but does not apply the aligned() type attribute to the
structure definition when used in combination with the section variable
attribute, choosing to align the structure on 32 bytes instead. My hunch is
thatthis is a gcc bug that appeared a few months before Fri Nov 14 17:47:47 2008
-0500, which led me to change the tracepoint structure alignment from 8 to 32
in commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 because of a very similar
problem.
One way to work around this could be to specify the aligned(8) type attribute to
the structure declarations *and* the aligned(8) variable attribute to the
structure definitions.
On 32-bit architectures, we really want a aligned(4), and on 64-bit
architectures, aligned(8). Represent this by creating:
#define __long_aligned __attribute__((__aligned__(__alignof__(long
The following patch passes the initial smoke test (it boots fine, without any
NULL pointer exception on x86_64).
(this patch is based on a 2.6.37 kernel)
Signed-off-by: Mathieu Desnoyers mathieu.desnoy...@efficios.com
---
include/linux/compiler.h |8 +---
include/linux/ftrace_event.h |2 +-
include/linux/syscalls.h | 20
include/trace/ftrace.h |8
include/trace/syscall.h |2 +-
kernel/trace/trace.h |2 +-
kernel/trace/trace_export.c |2 +-
7 files changed, 25 insertions(+), 19 deletions(-)
Index: linux-2.6-lttng/include/linux/compiler.h
===
--- linux-2.6-lttng.orig/include/linux/compiler.h
+++ linux-2.6-lttng/include/linux/compiler.h
@@ -57,6 +57,8 @@ extern void __chk_io_ptr(const volatile
# include linux/compiler-intel.h
#endif
+#define __long_aligned __attribute__((__aligned__(__alignof__(long
+
/*
* Generic compiler
Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod: Unknown relocation: 36
* Steven Rostedt (rost...@goodmis.org) wrote:
[ Added Mathieu on Cc, since he likes alignments ;-) ]
Oh yes, alignments are so much fun! (for some definitions of fun) ;)
On Sun, 2011-01-16 at 11:39 -0800, David Miller wrote:
From: Richard Mortimer ri...@oldelvet.org.uk
Date: Sun, 16 Jan 2011 14:17:49 +
I'm wondering if gcc is just getting better at honouring the source
code. The DEFINE_EVENT macros in include/trace/ftrace.h have a
__aligned__(4) attribute in them. Maybe that should be 8 on sparc64
systems.
The aligned 4 seems to be unchanged since include/trace/ftrace.h was
created in f42c85e74faa422cf0bc747ed808681145448f88 in April 2009.
That needs to be at least 8 on 64-bit systems. Why is this aligned
directive there at all?
IIRC, the problem showed up in 64-bit systems. OK, x86-64 (but of
course ;-).
The problem comes when the linker puts these sections together. We read
all the sections as one big array. If the linker puts in holes, then
this breaks the array, and the kernel crashes while reading the section.
I guess one solution is to remove the alignment at the allocation and
place it at the structure. This will mean all accesses to this structure
will need to be on an alignment.
The problem with these alignments is that they are just a hint to gcc, telling
it what the minimum alignment of a type should be. gcc is free to align on a
larger boundary if it wants to.
But the following test program is very instructive:
#include stdio.h
struct test {
void *a;
void *b;
void *c;
void *d;
void *e;
void *f;
void *g;
void *h;
void *i;
void *j;
void *k;
void *l;
void *m;
void *n;
void *o;
void *p;
void *q;
};
int main()
{
struct test __attribute__((aligned(4))) v;
printf(%d\n, __alignof__(v));
return 0;
}
(on x86_64, with gcc 4.5.1 and gcc 4.4.4)
if we put the __attribute__((aligned(4))) at the v definition (variable
attribute), the program returns an alignment of 4. If we move it after struct
test declaration (type attribute), the program returns an alignment of 8 (thus
taking the max between the attribute alignment and the largest field).
But that's a real problem, because in include/trace/ftrace.h, we have an
alignment of 4 forced on the definition, but there is a mismatch with
trace_events.c:
extern struct ftrace_event_call __start_ftrace_events[];
extern struct ftrace_event_call __stop_ftrace_events[];
for which the alignment attribute is missing (so an alignment of 8 will be
used there).
So it all worked as long as the size of struct ftrace_event_call was a multiple
of 8 bytes (struct ftrace_event_call constains 2 integers if we exclude the perf
fields), but the new fields added by perf contain a supplementary 4-byte
integer, which seems to be causing the breakage: the structures are appended one
next to another when defined, but the iteration on these structures thinks they
are 8-byte aligned.
Steven, what were you trying to fix in the first place when you added the
aligned(4) to the definition ? It might have just been that the _ftrace_events
section needed to be aligned on at least 8 bytes in the linker scripts, but was
only aligned on 4-bytes. Forcing the definition alignment down to 4 possibly
fixed the problem you experienced on x86_64, but seems to be causing other
problems.
I would recommend to:
- Keep the linker script _ftrace_events alignment as it is now (aligned on 32
bytes).
- Remove the aligned(4) attributes from all struct ftrace_event_call
definitions.
And see how this works. The only problem that might come up is if gcc decides to
align struct ftrace_event_call (which is about 136 bytes in size) on an
alignment larger than 32 bytes, which would be really surprising.
Mathieu
-- Steve
--
Mathieu Desnoyers
Operating System Efficiency RD Consultant
EfficiOS Inc.
http://www.efficios.com
--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110117193525.GD16154@Krystal
Reverts needed for 2.6.32.x ia64 percpu usage
Hi Greg, Pinging for a revert request for 2.6.32.x. It seems like we had a misunderstanding about exactly which patches should be reverted. The email below explains precisely which commits should be reverted from the 2.6.32.x stable series. Thanks, Mathieu - Forwarded message from Mathieu Desnoyers mathieu.desnoy...@efficios.com - Date: Thu, 27 May 2010 20:18:58 -0400 To: Ben Hutchings b...@decadent.org.uk, Greg Kroah-Hartman gre...@suse.de Cc: Tejun Heo t...@kernel.org, stable-rev...@kernel.org, debian-kernel@lists.debian.org User-Agent: Mutt/1.5.18 (2008-05-17) From: Mathieu Desnoyers mathieu.desnoy...@efficios.com Subject: Re: 2.6.32-stable percpu fixes * Ben Hutchings (b...@decadent.org.uk) wrote: These commits included in 2.6.32.12: ea0a09acd81c6d52c77d80f0d4089795df7bcb58 modules: fix incorrect percpu usage d150a2b96558a7349cbf3a72a279c37bc67d50fb module: fix __module_ref_addr() In addition to commit d150a2b96558a7349cbf3a72a279c37bc67d50fb, both commits: ea0a09acd81c6d52c77d80f0d4089795df7bcb58 modules: fix incorrect percpu usage b6b3dcd55e2327a968833ff3f22eda3b8dd7ef9e lockdep: fix incorrect percpu usage Should be reverted from the 2.6.32.x -stable series. Quoting the explanation from Tejun: I wrote on the bugzilla but this is not a compiler bug but the -stable patch [shouldn't; edit: should] have been applied only to 2.6.33. Not 2.6.32. This is because till 2.6.32, ia64 hadn't been converted to dynamic percpu allocator, so its static and dynamic percpu areas were separate and the per_cpu_ptr() wouldn't do the offsetting the module code expects there. So, please revert the patch from 2.6.32. Greg, it looks like we've not been clear enough about the fact that all three commits needed to be reverted from the 2.6.32.x stable branch. Sorry about that. Thanks, Mathieu apparently caused regressions, and have been reverted in SLE 11.1 and Debian unstable. The second has also now been reverted in 2.6.32.14, but the first has not. I'm afraid I don't understand the problems they were trying to solve, or the problems they caused, so could someone explain why the first should or not should not be reverted in 2.6.32-stable? (Matthieu previously asked whether it was really correct for 2.6.32: http://linux.kernel.org/pipermail/stable-review/2010-April/003571.html ) Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com - End forwarded message - -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com -- To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20100601143620.ga22...@krystal
Re: 2.6.32-stable percpu fixes
* Ben Hutchings (b...@decadent.org.uk) wrote: These commits included in 2.6.32.12: ea0a09acd81c6d52c77d80f0d4089795df7bcb58 modules: fix incorrect percpu usage d150a2b96558a7349cbf3a72a279c37bc67d50fb module: fix __module_ref_addr() In addition to commit d150a2b96558a7349cbf3a72a279c37bc67d50fb, both commits: ea0a09acd81c6d52c77d80f0d4089795df7bcb58 modules: fix incorrect percpu usage b6b3dcd55e2327a968833ff3f22eda3b8dd7ef9e lockdep: fix incorrect percpu usage Should be reverted from the 2.6.32.x -stable series. Quoting the explanation from Tejun: I wrote on the bugzilla but this is not a compiler bug but the -stable patch [shouldn't; edit: should] have been applied only to 2.6.33. Not 2.6.32. This is because till 2.6.32, ia64 hadn't been converted to dynamic percpu allocator, so its static and dynamic percpu areas were separate and the per_cpu_ptr() wouldn't do the offsetting the module code expects there. So, please revert the patch from 2.6.32. Greg, it looks like we've not been clear enough about the fact that all three commits needed to be reverted from the 2.6.32.x stable branch. Sorry about that. Thanks, Mathieu apparently caused regressions, and have been reverted in SLE 11.1 and Debian unstable. The second has also now been reverted in 2.6.32.14, but the first has not. I'm afraid I don't understand the problems they were trying to solve, or the problems they caused, so could someone explain why the first should or not should not be reverted in 2.6.32-stable? (Matthieu previously asked whether it was really correct for 2.6.32: http://linux.kernel.org/pipermail/stable-review/2010-April/003571.html ) Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. -- Mathieu Desnoyers Operating System Efficiency RD Consultant EfficiOS Inc. http://www.efficios.com signature.asc Description: Digital signature
