Skip to contentSkip to conversation

make section names compatible with -ffunction-sections -fdata-sections: parisc

PATCH In review Denys Vlasenko 4 participants 10 messages Jul 1, 2008 – Jul 6, 2008

Subsystems (2)
Generic include/asm header files PA-RISC architecture
lore
Share & tools
lore

Denys Vlasenko

author
#0 lore raw

PATCH 14/23 make section names compatible with -ffunction-sections -fdata-sections: parisc

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Signed-off-by Denys Vlasenko <[email protected]>

--
vda

arch/parisc/kernel/head.S +1 -1
@@ -345,7 +345,7 @@
 ENDPROC(stext)
 
 #ifndef CONFIG_64BIT
-	.section .data.read_mostly
+	.section .read_mostly.data
 
 	.align	4
 	.export	$global$,data
arch/parisc/kernel/init_task.c +4 -4
@@ -49,7 +49,7 @@
  * "init_task" linker map entry..
  */
 union thread_union init_thread_union
-	__attribute__((aligned(128))) __attribute__((__section__(".data.init_task"))) =
+	__attribute__((aligned(128))) __attribute__((__section__(".init_task.data"))) =
 		{ INIT_THREAD_INFO(init_task) };
 
 #if PT_NLEVELS == 3

@@ -58,11 +58,11 @@
  * guarantee that global objects will be laid out in memory in the same order 
  * as the order of declaration, so put these in different sections and use
  * the linker script to order them. */
-pmd_t pmd0[PTRS_PER_PMD] __attribute__ ((__section__ (".data.vm0.pmd"), aligned(PAGE_SIZE)));
+pmd_t pmd0[PTRS_PER_PMD] __attribute__ ((__section__ (".vm0.pmd.data"), aligned(PAGE_SIZE)));
 #endif
 
-pgd_t swapper_pg_dir[PTRS_PER_PGD] __attribute__ ((__section__ (".data.vm0.pgd"), aligned(PAGE_SIZE)));
-pte_t pg0[PT_INITIAL * PTRS_PER_PTE] __attribute__ ((__section__ (".data.vm0.pte"), aligned(PAGE_SIZE)));
+pgd_t swapper_pg_dir[PTRS_PER_PGD] __attribute__ ((__section__ (".vm0.pgd.data"), aligned(PAGE_SIZE)));
+pte_t pg0[PT_INITIAL * PTRS_PER_PTE] __attribute__ ((__section__ (".vm0.pte.data"), aligned(PAGE_SIZE)));
 
 /*
  * Initial task structure.
arch/parisc/kernel/vmlinux.lds.S +17 -17
@@ -54,10 +54,10 @@
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		*(.text.do_softirq)
-		*(.text.sys_exit)
-		*(.text.do_sigaltstack)
-		*(.text.do_fork)
+		*(.do_softirq.text)
+		*(.sys_exit.text)
+		*(.do_sigaltstack.text)
+		*(.do_fork.text)
 		*(.text.*)
 		*(.fixup)
 		*(.lock.text)		/* out-of-line lock text */

@@ -95,8 +95,8 @@
 
 	/* rarely changed data like cpu maps */
 	. = ALIGN(16);
-	.data.read_mostly : {
-		*(.data.read_mostly)
+	.read_mostly.data : {
+		*(.read_mostly.data)
 	}
 
 	. = ALIGN(L1_CACHE_BYTES);

@@ -107,14 +107,14 @@
 	}
 
 	. = ALIGN(L1_CACHE_BYTES);
-	.data.cacheline_aligned : {
-		*(.data.cacheline_aligned)
+	.cacheline_aligned.data : {
+		*(.cacheline_aligned.data)
 	}
 
 	/* PA-RISC locks requires 16-byte alignment */
 	. = ALIGN(16);
-	.data.lock_aligned : {
-		*(.data.lock_aligned)
+	.lock_aligned.data : {
+		*(.lock_aligned.data)
 	}
 
 	/* nosave data is really only used for software suspend...it's here

@@ -123,7 +123,7 @@
 	. = ALIGN(PAGE_SIZE);
 	__nosave_begin = .;
 	.data_nosave : {
-		*(.data.nosave)
+		*(.nosave.data)
 	}
 	. = ALIGN(PAGE_SIZE);
 	__nosave_end = .;

@@ -135,10 +135,10 @@
 	__bss_start = .;
 	/* page table entries need to be PAGE_SIZE aligned */
 	. = ALIGN(PAGE_SIZE);
-	.data.vmpages : {
-		*(.data.vm0.pmd)
-		*(.data.vm0.pgd)
-		*(.data.vm0.pte)
+	.data.vmpages : { /* TODO: rename .vmpages.data? */
+		*(.vm0.pmd.data)
+		*(.vm0.pgd.data)
+		*(.vm0.pte.data)
 	}
 	.bss : {
 		*(.bss)

@@ -150,8 +150,8 @@
 	/* assembler code expects init_task to be 16k aligned */
 	. = ALIGN(16384);
 	/* init_task */
-	.data.init_task : {
-		*(.data.init_task)
+	.init_task.data : {
+		*(.init_task.data)
 	}
 
 #ifdef CONFIG_64BIT
include/asm-parisc/cache.h +1 -1
@@ -28,7 +28,7 @@
 
 #define SMP_CACHE_BYTES L1_CACHE_BYTES
 
-#define __read_mostly __attribute__((__section__(".data.read_mostly")))
+#define __read_mostly __attribute__((__section__(".read_mostly.data")))
 
 void parisc_cache_init(void);	/* initializes cache-flushing */
 void disable_sr_hashing_asm(int); /* low level support for above */
include/asm-parisc/system.h +1 -1
@@ -174,7 +174,7 @@
 })
 
 #ifdef CONFIG_SMP
-# define __lock_aligned __attribute__((__section__(".data.lock_aligned")))
+# define __lock_aligned __attribute__((__section__(".lock_aligned.data")))
 #endif
 
 #define arch_align_stack(x) (x)
maintainer
#1 lore raw
Quoted 5 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

arch/parisc/kernel/vmlinux.lds.S line 60

Quoted 8 lines

- *(.text.do_softirq)

arch/parisc/kernel/vmlinux.lds.S +4 -3
-               *(.text.sys_exit)
-               *(.text.do_sigaltstack)
-               *(.text.do_fork)
+               *(.do_softirq.text)
+               *(.sys_exit.text)
+               *(.do_sigaltstack.text)
+               *(.do_fork.text)

And thus by the same token the data transformations.

James

author
#2 lore raw
Quoted 5 lines · James Bottomley
Quoted 4 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Quoted 17 lines
Quoted 1 line
Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

Quoted 8 lines
arch/parisc/kernel/vmlinux.lds.S +4 -4
-               *(.text.do_softirq)
-               *(.text.sys_exit)
-               *(.text.do_sigaltstack)
-               *(.text.do_fork)
+               *(.do_softirq.text)
+               *(.sys_exit.text)
+               *(.do_sigaltstack.text)
+               *(.do_fork.text)

arch/parisc/kernel/vmlinux.lds.S contains these lines:

               TEXT_TEXT
               SCHED_TEXT
               LOCK_TEXT
               *(.text.do_softirq)
               *(.text.sys_exit)
               *(.text.do_sigaltstack)
               *(.text.do_fork)

which suggested to me that for parisc it is important to have these sections in that place (after LOCK_TEXT) and order.

If you use -ffunction-sections, any function with the name do_fork (say, a static function somewhere) will end up in .text.do_fork function, and will be "mixed up" with global do_fork(). For parisc it is maybe not a problem (I am not an expert) but in other places/arches people clearly would not want this kind of things to happen.

In order to handle these situations uniformly, in these patches I decided to _never_ use .text.XXXX names for sections, effectively leaving them "reserved for gcc's use".

Did I understand you right that in this chunk I need to leave .text.FUNC_NAME as it was before?

Quoted 1 line

And thus by the same token the data transformations.

It would be easiest for me if you will reply to the parisc patch and indicate all parts where I should NOT do name change.

Thanks!
--
vda

maintainer
#3 lore raw
Quoted 34 lines · Denys Vlasenko
Quoted 5 lines · James Bottomley
Quoted 4 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Quoted 17 lines
Quoted 1 line
Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

Quoted 8 lines
arch/parisc/kernel/vmlinux.lds.S +4 -4
-               *(.text.do_softirq)
-               *(.text.sys_exit)
-               *(.text.do_sigaltstack)
-               *(.text.do_fork)
+               *(.do_softirq.text)
+               *(.sys_exit.text)
+               *(.do_sigaltstack.text)
+               *(.do_fork.text)

arch/parisc/kernel/vmlinux.lds.S contains these lines:

               TEXT_TEXT
               SCHED_TEXT
               LOCK_TEXT
               *(.text.do_softirq)
               *(.text.sys_exit)
               *(.text.do_sigaltstack)
               *(.text.do_fork)
Quoted 2 lines

which suggested to me that for parisc it is important to have these sections in that place (after LOCK_TEXT) and order.

Yes ... moderately.

Quoted 6 lines

If you use -ffunction-sections, any function with the name do_fork (say, a static function somewhere) will end up in .text.do_fork function, and will be "mixed up" with global do_fork(). For parisc it is maybe not a problem (I am not an expert) but in other places/arches people clearly would not want this kind of things to happen.

Erm, but we're trying to name text sections of the -ffunction-sections. We're doing this because we are trying to do a semblance of a reasonable arrangement for the relative jumps (and avoid stubs). Your patch is explicitly breaking all of this.

Quoted 3 lines

In order to handle these situations uniformly, in these patches I decided to _never_ use .text.XXXX names for sections, effectively leaving them "reserved for gcc's use".

We need to use the names gcc outputs, not some random name.

Quoted 2 lines

Did I understand you right that in this chunk I need to leave .text.FUNC_NAME as it was before?

Yes.

Quoted 4 lines
Quoted 1 line

And thus by the same token the data transformations.

It would be easiest for me if you will reply to the parisc patch and indicate all parts where I should NOT do name change.

That would be the piece above.

James

#4 lore raw
In reply to Denys Vlasenko
Quoted 6 lines · Denys Vlasenko
Quoted 5 lines · James Bottomley
Quoted 4 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

We've been building FR-V kernels with --gc-sections for a long time, too.

Quoted 3 lines

In order to handle these situations uniformly, in these patches I decided to _never_ use .text.XXXX names for sections, effectively leaving them "reserved for gcc's use".

It makes a certain amount of sense to do this uniformly for all architectures -- leaving .text.* and .data.* for GCC, and using something else whenever we manually name sections.

Denys' patch for parisc would need a little more thought, but it's probably worth it to be consistent. Especially if we can move more stuff out of athe arch-specific linker scripts and into <asm-generic/vmlinux.lds.h>

Signature · 1 line
dwmw2
#5 lore raw
Quoted 13 lines · David Woodhouse
Quoted 12 lines · Denys Vlasenko
Quoted 11 lines · James Bottomley
Quoted 5 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

We've been building FR-V kernels with --gc-sections for a long time, too.
...

Is there any specific reason why it's not done in the upstream kernel?

Quoted 1 line

dwmw2

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#6 lore raw
Quoted 14 lines · Adrian Bunk
Quoted 13 lines · David Woodhouse
Quoted 12 lines · Denys Vlasenko
Quoted 11 lines · James Bottomley
Quoted 5 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

We've been building FR-V kernels with --gc-sections for a long time, too.
...

Is there any specific reason why it's not done in the upstream kernel?

Isn't it? I thought it was.

Or maybe we were only doing that before we added MMU support. I remember the exception tables complicate matters a little.

Signature · 1 line
dwmw2
#7 lore raw
Quoted 18 lines · David Woodhouse
Quoted 17 lines · Adrian Bunk
Quoted 16 lines · David Woodhouse
Quoted 14 lines · Denys Vlasenko
Quoted 13 lines · James Bottomley
Quoted 5 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Signed-off-by Denys Vlasenko <[email protected]>

Um ... if you look at the Makefile you'll see we already build parisc with -ffunction-sections; we have to: our relative jumps are too small to guarantee finding the stubs in large files.

Since our text is -ffunction-sections compatible already, I question the need for transformations like this:

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

We've been building FR-V kernels with --gc-sections for a long time, too.
...

Is there any specific reason why it's not done in the upstream kernel?

Isn't it? I thought it was.
...

It is in the Makefile, but only enabled through a nonexisting CONFIG_GC_SECTIONS option.

Quoted 1 line

dwmw2

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
author
#8 lore raw
In reply to Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Updated version - does not try to rename sections
which really are expected to be produced by gcc - .text.sys_exit etc.

James please ACK/NAK.

Signed-off-by Denys Vlasenko <[email protected]>

--
vda

arch/parisc/kernel/head.S +1 -1
@@ -345,7 +345,7 @@
 ENDPROC(stext)
 
 #ifndef CONFIG_64BIT
-	.section .data.read_mostly
+	.section .read_mostly.data
 
 	.align	4
 	.export	$global$,data
arch/parisc/kernel/init_task.c +4 -4
@@ -49,7 +49,7 @@
  * "init_task" linker map entry..
  */
 union thread_union init_thread_union
-	__attribute__((aligned(128))) __attribute__((__section__(".data.init_task"))) =
+	__attribute__((aligned(128))) __attribute__((__section__(".init_task.data"))) =
 		{ INIT_THREAD_INFO(init_task) };
 
 #if PT_NLEVELS == 3

@@ -58,11 +58,11 @@
  * guarantee that global objects will be laid out in memory in the same order 
  * as the order of declaration, so put these in different sections and use
  * the linker script to order them. */
-pmd_t pmd0[PTRS_PER_PMD] __attribute__ ((__section__ (".data.vm0.pmd"), aligned(PAGE_SIZE)));
+pmd_t pmd0[PTRS_PER_PMD] __attribute__ ((__section__ (".vm0.pmd.data"), aligned(PAGE_SIZE)));
 #endif
 
-pgd_t swapper_pg_dir[PTRS_PER_PGD] __attribute__ ((__section__ (".data.vm0.pgd"), aligned(PAGE_SIZE)));
-pte_t pg0[PT_INITIAL * PTRS_PER_PTE] __attribute__ ((__section__ (".data.vm0.pte"), aligned(PAGE_SIZE)));
+pgd_t swapper_pg_dir[PTRS_PER_PGD] __attribute__ ((__section__ (".vm0.pgd.data"), aligned(PAGE_SIZE)));
+pte_t pg0[PT_INITIAL * PTRS_PER_PTE] __attribute__ ((__section__ (".vm0.pte.data"), aligned(PAGE_SIZE)));
 
 /*
  * Initial task structure.
arch/parisc/kernel/vmlinux.lds.S +13 -13
@@ -95,8 +95,8 @@
 
 	/* rarely changed data like cpu maps */
 	. = ALIGN(16);
-	.data.read_mostly : {
-		*(.data.read_mostly)
+	.read_mostly.data : {
+		*(.read_mostly.data)
 	}
 
 	. = ALIGN(L1_CACHE_BYTES);

@@ -107,14 +107,14 @@
 	}
 
 	. = ALIGN(L1_CACHE_BYTES);
-	.data.cacheline_aligned : {
-		*(.data.cacheline_aligned)
+	.cacheline_aligned.data : {
+		*(.cacheline_aligned.data)
 	}
 
 	/* PA-RISC locks requires 16-byte alignment */
 	. = ALIGN(16);
-	.data.lock_aligned : {
-		*(.data.lock_aligned)
+	.lock_aligned.data : {
+		*(.lock_aligned.data)
 	}
 
 	/* nosave data is really only used for software suspend...it's here

@@ -123,7 +123,7 @@
 	. = ALIGN(PAGE_SIZE);
 	__nosave_begin = .;
 	.data_nosave : {
-		*(.data.nosave)
+		*(.nosave.data)
 	}
 	. = ALIGN(PAGE_SIZE);
 	__nosave_end = .;

@@ -135,10 +135,10 @@
 	__bss_start = .;
 	/* page table entries need to be PAGE_SIZE aligned */
 	. = ALIGN(PAGE_SIZE);
-	.data.vmpages : {
-		*(.data.vm0.pmd)
-		*(.data.vm0.pgd)
-		*(.data.vm0.pte)
+	.data.vmpages : { /* TODO: rename .vmpages.data? */
+		*(.vm0.pmd.data)
+		*(.vm0.pgd.data)
+		*(.vm0.pte.data)
 	}
 	.bss : {
 		*(.bss)

@@ -150,8 +150,8 @@
 	/* assembler code expects init_task to be 16k aligned */
 	. = ALIGN(16384);
 	/* init_task */
-	.data.init_task : {
-		*(.data.init_task)
+	.init_task.data : {
+		*(.init_task.data)
 	}
 
 #ifdef CONFIG_64BIT
include/asm-parisc/cache.h +1 -1
@@ -28,7 +28,7 @@
 
 #define SMP_CACHE_BYTES L1_CACHE_BYTES
 
-#define __read_mostly __attribute__((__section__(".data.read_mostly")))
+#define __read_mostly __attribute__((__section__(".read_mostly.data")))
 
 void parisc_cache_init(void);	/* initializes cache-flushing */
 void disable_sr_hashing_asm(int); /* low level support for above */
include/asm-parisc/system.h +1 -1
@@ -174,7 +174,7 @@
 })
 
 #ifdef CONFIG_SMP
-# define __lock_aligned __attribute__((__section__(".data.lock_aligned")))
+# define __lock_aligned __attribute__((__section__(".lock_aligned.data")))
 #endif
 
 #define arch_align_stack(x) (x)
maintainer
#9 lore raw
Quoted 6 lines · Denys Vlasenko

The purpose of this patch is to make kernel buildable with "gcc -ffunction-sections -fdata-sections". This patch fixes parisc architecture.

Updated version - does not try to rename sections
which really are expected to be produced by gcc - .text.sys_exit etc.

Actually, I've thought about it some more and I don't think your approach is quite right. The way gcc works with -ffunction-sections makes it clear that it expects the section namespace to be scoped left to right. However, by using *.text and *.data, your patch is following a reverse convention (putting your most global name to the right). This is setting us up for a clash with future global section names that our toolchain might emit in future.

Since we have to have special handling for your reversed sections in every vmlinux.lds, what about introducing our own namespace instead? So we reserve the global section specifier linux and then make everything a namespace under this. So our constructed text and data sections become

linux.text.*

and

linux.data.*

That way we could only ever get a clash if the toolchain decided to use a section global name linux (which we can beat them up over).

James