diff mbox

[RFT/RFC,6/6] ARM: keep .text and .fixup regions together

Message ID 1426181892-15440-7-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel March 12, 2015, 5:38 p.m. UTC
Fixup snippets are put into a dedicated section so that they don't
bloat cache lines with instructions that are usually not executed.
But there is no reason to put all these snippets together at the far
end of the .text output region, where the branch instruction they
contain could go out of range if the kernel grows in size.

Instead, emit .text and .fixup regions together for each input object.
They should still be out of the way, but not so far that they go out
of range.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

Note that the TEXT_TEXT macro will emit *(.text) again but this should be
harmless.

 arch/arm/kernel/vmlinux.lds.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Nicolas Pitre March 12, 2015, 8:34 p.m. UTC | #1
On Thu, 12 Mar 2015, Ard Biesheuvel wrote:

> Fixup snippets are put into a dedicated section so that they don't
> bloat cache lines with instructions that are usually not executed.
> But there is no reason to put all these snippets together at the far
> end of the .text output region, where the branch instruction they
> contain could go out of range if the kernel grows in size.
> 
> Instead, emit .text and .fixup regions together for each input object.
> They should still be out of the way, but not so far that they go out
> of range.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Acked-by: Nicolas Pitre <nico@linaro.org>

> ---
> 
> Note that the TEXT_TEXT macro will emit *(.text) again but this should be
> harmless.
> 
>  arch/arm/kernel/vmlinux.lds.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 2e7b2220ef5f..01630c38fd6c 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -108,13 +108,13 @@ SECTIONS
>  			*(.exception.text)
>  			__exception_text_end = .;
>  			IRQENTRY_TEXT
> +#ifdef CONFIG_MMU
> +			*(.text .fixup)
> +#endif
>  			TEXT_TEXT
>  			SCHED_TEXT
>  			LOCK_TEXT
>  			KPROBES_TEXT
> -#ifdef CONFIG_MMU
> -			*(.fixup)
> -#endif
>  			*(.gnu.warning)
>  			*(.glue_7)
>  			*(.glue_7t)
> -- 
> 1.8.3.2
> 
>
Ard Biesheuvel March 12, 2015, 9:18 p.m. UTC | #2
On 12 March 2015 at 22:10, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Mar 12, 2015 at 06:38:12PM +0100, Ard Biesheuvel wrote:
>> Fixup snippets are put into a dedicated section so that they don't
>> bloat cache lines with instructions that are usually not executed.
>> But there is no reason to put all these snippets together at the far
>> end of the .text output region, where the branch instruction they
>> contain could go out of range if the kernel grows in size.
>>
>> Instead, emit .text and .fixup regions together for each input object.
>> They should still be out of the way, but not so far that they go out
>> of range.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>
>> Note that the TEXT_TEXT macro will emit *(.text) again but this should be
>> harmless.
>
> However, I wonder if by doing this, we're weakening the ability for
> kallsyms final link to succeed:
>
> /* .text section. Map to function alignment to avoid address changes
>  * during second ld run in second ld pass when generating System.map */
>
> Can we not just move .fixup before TEXT_TEXT?  The only thing between it
> and .text would be .text.hot.
>

Putting .fixup before .text already helps, but not enough for the
.config Arnd gave me that I have been testing this with.

What *(.text .fixup) does (i.e., putting both section names inside the
parentheses), is emitting both sections for each input object file, so
they will always be close to the object that it refers to, so it is
not the same thing.
Ard Biesheuvel March 13, 2015, 11:26 a.m. UTC | #3
On 13 March 2015 at 12:18, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 12 March 2015 21:22:02 Russell King - ARM Linux wrote:
>> On Thu, Mar 12, 2015 at 10:18:26PM +0100, Ard Biesheuvel wrote:
>> > >> Note that the TEXT_TEXT macro will emit *(.text) again but this should be
>> > >> harmless.
>> > >
>> > > However, I wonder if by doing this, we're weakening the ability for
>> > > kallsyms final link to succeed:
>> > >
>> > > /* .text section. Map to function alignment to avoid address changes
>> > >  * during second ld run in second ld pass when generating System.map */
>> > >
>> > > Can we not just move .fixup before TEXT_TEXT?  The only thing between it
>> > > and .text would be .text.hot.
>> > >
>> >
>> > Putting .fixup before .text already helps, but not enough for the
>> > .config Arnd gave me that I have been testing this with.
>> >
>> > What *(.text .fixup) does (i.e., putting both section names inside the
>> > parentheses), is emitting both sections for each input object file, so
>> > they will always be close to the object that it refers to, so it is
>> > not the same thing.
>>
>> I'll suggest a different solution then - how about modifying
>> asm-generic/vmlinux.lds.h to change *(.text) to *(.text .text.fixup)
>> and we move all the .fixup sections to .text.fixup ?  Arnd?
>
> No objections from me, but I really don't know enough about the possible
> side-effects this may have on the other architectures, so we need to
> run this by the linux-arch mailing list.
>

I don't think it could affect any other architecture if nobody is
populating .text.fixup yet.
diff mbox

Patch

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 2e7b2220ef5f..01630c38fd6c 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -108,13 +108,13 @@  SECTIONS
 			*(.exception.text)
 			__exception_text_end = .;
 			IRQENTRY_TEXT
+#ifdef CONFIG_MMU
+			*(.text .fixup)
+#endif
 			TEXT_TEXT
 			SCHED_TEXT
 			LOCK_TEXT
 			KPROBES_TEXT
-#ifdef CONFIG_MMU
-			*(.fixup)
-#endif
 			*(.gnu.warning)
 			*(.glue_7)
 			*(.glue_7t)