Subject: Re: [PATCH 2/3] jump label: move jump table to r/w section
From: Steven Rostedt <rostedt@goodmis.org>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Jason Baron <jbaron@redhat.com>, mingo@elte.hu, peterz@infradead.org,
        hpa@zytor.com, tglx@linutronix.de, andi@firstfloor.org,
        roland@redhat.com, rth@redhat.com, masami.hiramatsu.pt@hitachi.com,
        fweisbec@gmail.com, avi@redhat.com, davem@davemloft.net,
        sam@ravnborg.org, ddaney@caviumnetworks.com, michael@ellerman.id.au,
        linux-kernel@vger.kernel.org
In-Reply-To: <20101123235519.GB21549@Krystal>
References: <cover.1290546539.git.jbaron@redhat.com>
	 <bbc34a86a4cf4addcde1b85d7fa7faecc0966c03.1290546539.git.jbaron@redhat.com>
	 <20101123235519.GB21549@Krystal>
Content-Type: text/plain; charset="ISO-8859-15"
Date: Tue, 23 Nov 2010 21:18:53 -0500
Message-ID: <1290565133.30543.430.camel@gandalf.stny.rr.com>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2810
Lines: 78

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.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/