Messages in this thread | | | Subject | Re: [PATCH 2/3] jump label: move jump table to r/w section | From | Steven Rostedt <> | Date | Tue, 23 Nov 2010 21:59:53 -0500 |
| |
[ Calling attention to David Miller ]
On Tue, 2010-11-23 at 21:18 -0500, Steven Rostedt wrote: > On Tue, 2010-11-23 at 18:55 -0500, Mathieu Desnoyers wrote: > > * Jason Baron (jbaron@redhat.com) wrote: > > > Since we writing the jump table it should be be in R/W kernel > > > section. Move it to DATA_DATA > > > > > > Signed-off-by: Jason Baron <jbaron@redhat.com> > > > --- > > > include/asm-generic/vmlinux.lds.h | 14 ++++---------- > > > 1 files changed, 4 insertions(+), 10 deletions(-) > > > > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > > > index bd69d79..9ca894d 100644 > > > --- a/include/asm-generic/vmlinux.lds.h > > > +++ b/include/asm-generic/vmlinux.lds.h > > > @@ -161,6 +161,10 @@ > > > VMLINUX_SYMBOL(__start___tracepoints) = .; \ > > > *(__tracepoints) \ > > > VMLINUX_SYMBOL(__stop___tracepoints) = .; \ > > > + . = ALIGN(8); \ > > > > Past churn with various architectures and compiler with tracepoints, > > markers and immediate values lead me to hint at the following approach > > for jump label structure alignment: > > > > . = ALIGN(32); > > > > and to modify jump_label.h to have: > > > > struct jump_entry { > > jump_label_t code; > > jump_label_t target; > > jump_label_t key; > > } __attribute__((aligned(32))); > > > > Otherwise, the compiler is free to choose on which value it prefers to > > align the jump_entry structures, which might not match the address at > > which the linker scripts puts the beginning of the jump table. > > > > In this case, given that we put put the jump label table after the > > tracepoint table, we should be already aligned on 32 bytes. But I would > > recommend to put the . = ALIGN(32) in the linker script anyway, just for > > documentation purpose (and it should not add any padding in this case). > > > > This is not a problem introduced by this patch, it also applies to the > > current jump label code. > > > > Looking at this we have much bigger issues. That alignment to the > structure wont do anything but break things. This is because the > structure is not used in assigning that section. It's done in assembly: > > # define JUMP_LABEL(key, label) \ > do { \ > asm goto("1:" \ > JUMP_LABEL_INITIAL_NOP \ > ".pushsection __jump_table, \"a\" \n\t"\ > _ASM_PTR "1b, %l[" #label "], %c0 \n\t" \ > ".popsection \n\t" \ > : : "i" (key) : : label); \ > } while (0) > > > > That __ASM_PTR "1b, %l[" #label "], %c0 \n\t" is assigning the section > the address of label 1, the address of the label #label, and the key. > > We either need to fix this by allocating an array of jump_entrys and > having each arch copy the data into it, or we need to do something > similar to exception tables.
Talking with Mathieu about this, we may have the simple solution of adding the packed attribute to all arch declarations of struct jump_entry, and that should fix it. And having this on an 8 byte alignment should be fine.
I just want to make sure this is fine with sparc, as it is one of the more exotic archs.
-- Steve
| |