[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

In trying to build numerous targets, I've managed to reproduce the
problem with KALLSYMS enough to come up with some ideas for a fix.
Neither of these fixes affects the size of the kernel.  I've compile
tested all of the defconfigs that are present in the kernel, and
neither causes any new compilation failures (several don't build).

The first fix adjusts the alignment of the per-cpu section, in the
cases where it is blank.  If it has the same alignment as the
following section, there won't be inconsistencies in the position when
the kallsyms data is inserted.

The second fix just eliminates the per_cpu section on non-smp builds.

Either case only affects the position of an empty section, and should
have no effect on the position or size of anything else.

I think the second patch (eliminate the per-cpu section) is cleaner,
but I'm concerned about something I've missed or changed.

The second patch does make the per-cpu section smaller (since some
symbols are left out).  It also affects the position of init_end, and
may need a page alignment before it.  But, this would also suggest
that currently, on SMP, there might be a page that isn't being freed.

David Brown (1):
  ARM: Prevent KALLSYM size mismatch on ARM.

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

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

In trying to build numerous targets, I've managed to reproduce the
problem with KALLSYMS enough to come up with some ideas for a fix.
Neither of these fixes affects the size of the kernel.  I've compile
tested all of the defconfigs that are present in the kernel, and
neither causes any new compilation failures (several don't build).

The first fix adjusts the alignment of the per-cpu section, in the
cases where it is blank.  If it has the same alignment as the
following section, there won't be inconsistencies in the position when
the kallsyms data is inserted.

The second fix just eliminates the per_cpu section on non-smp builds.

Either case only affects the position of an empty section, and should
have no effect on the position or size of anything else.

I think the second patch (eliminate the per-cpu section) is cleaner,
but I'm concerned about something I've missed or changed.

The second patch does make the per-cpu section smaller (since some
symbols are left out).  It also affects the position of init_end, and
may need a page alignment before it.  But, this would also suggest
that currently, on SMP, there might be a page that isn't being freed.

David Brown (1):
  ARM: Prevent KALLSYM size mismatch on ARM.

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

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH variant 1] ARM: Prevent KALLSYM size mismatch on ARM.

[David Brown]

ARM builds seem to be plagued by an occasional build error:

    Inconsistent kallsyms data
    This is a bug - please report about it
    Try "make KALLSYMS_EXTRA_PASS=1" as a workaround

The problem has to do with alignment of some sections by the linker.
The kallsyms data is built in two passes by first linking the kernel
without it, and then linking the kernel again with the symbols
included.  Normally, this just shifts the symbols, without changing
their order, and the compression used by the kallsyms gives the same
result.

On non SMP, the per CPU data is empty.  Depending on the where the
alignment ends up, it can come out as either:

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

or

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* no padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

if the alignment satisfies both.  Because symbols that have the same
address are sorted by 'nm -n', the second case will be in a different
order than the first case.  This changes the compression, changing the
size of the kallsym data, causing the build failure.

The KALLSYMS_EXTRA_PASS=1 workaround usually works, but it is still
possible to have the alignment change between the second and third
pass.  It's probably even possible for it to never reach a fixedpoint.

The problem only occurs on non-SMP, when the per-cpu data is empty,
and when the data segment has alignment (and immediately follows the
text segments).  In these cases, add the THREAD_SIZE alignment above
the per-cpu data so that the empty segment will always be adjacent to
the data segment.

This shouldn't ever change the size of the kernel, and only should
affect the location of the per_cpu symbols, which contain no data.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/kernel/vmlinux.lds.S |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 43a31fb..f3778c7 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -183,6 +183,9 @@ SECTIONS
 	}
 #endif
 
+#if !defined(CONFIG_SMP) && !defined(CONFIG_XIP_KERNEL)
+	. = ALIGN(THREAD_SIZE);
+#endif
 	PERCPU_SECTION(L1_CACHE_BYTES)
 
 #ifdef CONFIG_XIP_KERNEL
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH variant 1] ARM: Prevent KALLSYM size mismatch on ARM.

[David Brown]

ARM builds seem to be plagued by an occasional build error:

    Inconsistent kallsyms data
    This is a bug - please report about it
    Try "make KALLSYMS_EXTRA_PASS=1" as a workaround

The problem has to do with alignment of some sections by the linker.
The kallsyms data is built in two passes by first linking the kernel
without it, and then linking the kernel again with the symbols
included.  Normally, this just shifts the symbols, without changing
their order, and the compression used by the kallsyms gives the same
result.

On non SMP, the per CPU data is empty.  Depending on the where the
alignment ends up, it can come out as either:

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

or

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* no padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

if the alignment satisfies both.  Because symbols that have the same
address are sorted by 'nm -n', the second case will be in a different
order than the first case.  This changes the compression, changing the
size of the kallsym data, causing the build failure.

The KALLSYMS_EXTRA_PASS=1 workaround usually works, but it is still
possible to have the alignment change between the second and third
pass.  It's probably even possible for it to never reach a fixedpoint.

The problem only occurs on non-SMP, when the per-cpu data is empty,
and when the data segment has alignment (and immediately follows the
text segments).  In these cases, add the THREAD_SIZE alignment above
the per-cpu data so that the empty segment will always be adjacent to
the data segment.

This shouldn't ever change the size of the kernel, and only should
affect the location of the per_cpu symbols, which contain no data.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/kernel/vmlinux.lds.S |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 43a31fb..f3778c7 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -183,6 +183,9 @@ SECTIONS
 	}
 #endif
 
+#if !defined(CONFIG_SMP) && !defined(CONFIG_XIP_KERNEL)
+	. = ALIGN(THREAD_SIZE);
+#endif
 	PERCPU_SECTION(L1_CACHE_BYTES)
 
 #ifdef CONFIG_XIP_KERNEL
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH variant 2] ARM: Prevent KALLSYM size mismatch on ARM.

[David Brown]

ARM builds seem to be plagued by an occasional build error:

    Inconsistent kallsyms data
    This is a bug - please report about it
    Try "make KALLSYMS_EXTRA_PASS=1" as a workaround

The problem has to do with alignment of some sections by the linker.
The kallsyms data is built in two passes by first linking the kernel
without it, and then linking the kernel again with the symbols
included.  Normally, this just shifts the symbols, without changing
their order, and the compression used by the kallsyms gives the same
result.

On non SMP, the per CPU data is empty.  Depending on the where the
alignment ends up, it can come out as either:

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

or

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* no padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

if the alignment satisfies both.  Because symbols that have the same
address are sorted by 'nm -n', the second case will be in a different
order than the first case.  This changes the compression, changing the
size of the kallsym data, causing the build failure.

The KALLSYMS_EXTRA_PASS=1 workaround usually works, but it is still
possible to have the alignment change between the second and third
pass.  It's probably even possible for it to never reach a fixedpoint.

The problem only occurs on non-SMP, when the per-cpu data is empty,
and when the data segment has alignment (and immediately follows the
text segments).  Fix this by only including the per_cpu section on
SMP, when it is not empty.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/kernel/vmlinux.lds.S |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 43a31fb..36ff15b 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -183,7 +183,9 @@ SECTIONS
 	}
 #endif
 
+#ifdef CONFIG_SMP
 	PERCPU_SECTION(L1_CACHE_BYTES)
+#endif
 
 #ifdef CONFIG_XIP_KERNEL
 	__data_loc = ALIGN(4);		/* location in binary */
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH variant 2] ARM: Prevent KALLSYM size mismatch on ARM.

[David Brown]

ARM builds seem to be plagued by an occasional build error:

    Inconsistent kallsyms data
    This is a bug - please report about it
    Try "make KALLSYMS_EXTRA_PASS=1" as a workaround

The problem has to do with alignment of some sections by the linker.
The kallsyms data is built in two passes by first linking the kernel
without it, and then linking the kernel again with the symbols
included.  Normally, this just shifts the symbols, without changing
their order, and the compression used by the kallsyms gives the same
result.

On non SMP, the per CPU data is empty.  Depending on the where the
alignment ends up, it can come out as either:

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

or

   +-------------------+
   | last text segment |
   +-------------------+
   /* padding */
   +-------------------+     <- L1_CACHE_BYTES alignemnt
   | per cpu (empty)   |
   +-------------------+
__per_cpu_end:
   /* no padding */
__data_loc:
   +-------------------+     <- THREAD_SIZE alignment
   | data              |
   +-------------------+

if the alignment satisfies both.  Because symbols that have the same
address are sorted by 'nm -n', the second case will be in a different
order than the first case.  This changes the compression, changing the
size of the kallsym data, causing the build failure.

The KALLSYMS_EXTRA_PASS=1 workaround usually works, but it is still
possible to have the alignment change between the second and third
pass.  It's probably even possible for it to never reach a fixedpoint.

The problem only occurs on non-SMP, when the per-cpu data is empty,
and when the data segment has alignment (and immediately follows the
text segments).  Fix this by only including the per_cpu section on
SMP, when it is not empty.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/kernel/vmlinux.lds.S |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 43a31fb..36ff15b 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -183,7 +183,9 @@ SECTIONS
 	}
 #endif
 
+#ifdef CONFIG_SMP
 	PERCPU_SECTION(L1_CACHE_BYTES)
+#endif
 
 #ifdef CONFIG_XIP_KERNEL
 	__data_loc = ALIGN(4);		/* location in binary */
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:

> David Brown (1):
>   ARM: Prevent KALLSYM size mismatch on ARM.
> 
>  arch/arm/kernel/vmlinux.lds.S |    3 +++
>  1 file changed, 3 insertions(+)

Just wondering if anyone has had a chance to look at either of these,
or try them.  I haven't seen any KALLSYMS mismatch build errors with
either of these patches applied.

Thanks,
David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:

> David Brown (1):
>   ARM: Prevent KALLSYM size mismatch on ARM.
> 
>  arch/arm/kernel/vmlinux.lds.S |    3 +++
>  1 file changed, 3 insertions(+)

Just wondering if anyone has had a chance to look at either of these,
or try them.  I haven't seen any KALLSYMS mismatch build errors with
either of these patches applied.

Thanks,
David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] ARM: two possible fixes for the KALLSYMS build problem

[Russell King - ARM Linux]

On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> 
> > David Brown (1):
> >   ARM: Prevent KALLSYM size mismatch on ARM.
> > 
> >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> >  1 file changed, 3 insertions(+)
> 
> Just wondering if anyone has had a chance to look at either of these,
> or try them.  I haven't seen any KALLSYMS mismatch build errors with
> either of these patches applied.

I think variant 2 is the better approach out of either as it doesn't add
to the size of the resulting kernel image.

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[Russell King - ARM Linux]

On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> 
> > David Brown (1):
> >   ARM: Prevent KALLSYM size mismatch on ARM.
> > 
> >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> >  1 file changed, 3 insertions(+)
> 
> Just wondering if anyone has had a chance to look at either of these,
> or try them.  I haven't seen any KALLSYMS mismatch build errors with
> either of these patches applied.

I think variant 2 is the better approach out of either as it doesn't add
to the size of the resulting kernel image.

Re: [PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

On Tue, Jun 12, 2012 at 03:32:49PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> > On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> > 
> > > David Brown (1):
> > >   ARM: Prevent KALLSYM size mismatch on ARM.
> > > 
> > >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> > >  1 file changed, 3 insertions(+)
> > 
> > Just wondering if anyone has had a chance to look at either of these,
> > or try them.  I haven't seen any KALLSYMS mismatch build errors with
> > either of these patches applied.
> 
> I think variant 2 is the better approach out of either as it doesn't add
> to the size of the resulting kernel image.

The first variant doesn't ever increase the size of the kernel,
either.  Variant two actually makes it a little smaller, since a few
symbols are eliminated.

The second variant is cleaner, though as long as it is safe in every
configuration.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[David Brown]

On Tue, Jun 12, 2012 at 03:32:49PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> > On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> > 
> > > David Brown (1):
> > >   ARM: Prevent KALLSYM size mismatch on ARM.
> > > 
> > >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> > >  1 file changed, 3 insertions(+)
> > 
> > Just wondering if anyone has had a chance to look at either of these,
> > or try them.  I haven't seen any KALLSYMS mismatch build errors with
> > either of these patches applied.
> 
> I think variant 2 is the better approach out of either as it doesn't add
> to the size of the resulting kernel image.

The first variant doesn't ever increase the size of the kernel,
either.  Variant two actually makes it a little smaller, since a few
symbols are eliminated.

The second variant is cleaner, though as long as it is safe in every
configuration.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] ARM: two possible fixes for the KALLSYMS build problem

[Russell King - ARM Linux]

On Tue, Jun 12, 2012 at 09:02:29AM -0700, David Brown wrote:
> On Tue, Jun 12, 2012 at 03:32:49PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> > > On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> > > 
> > > > David Brown (1):
> > > >   ARM: Prevent KALLSYM size mismatch on ARM.
> > > > 
> > > >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> > > >  1 file changed, 3 insertions(+)
> > > 
> > > Just wondering if anyone has had a chance to look at either of these,
> > > or try them.  I haven't seen any KALLSYMS mismatch build errors with
> > > either of these patches applied.
> > 
> > I think variant 2 is the better approach out of either as it doesn't add
> > to the size of the resulting kernel image.
> 
> The first variant doesn't ever increase the size of the kernel,
> either.  Variant two actually makes it a little smaller, since a few
> symbols are eliminated.

Ok.

> The second variant is cleaner, though as long as it is safe in every
> configuration.

Well, we don't have any per-CPU data unless we're building for SMP.
See:

#ifndef PER_CPU_BASE_SECTION
#ifdef CONFIG_SMP
#define PER_CPU_BASE_SECTION ".data..percpu"
#else
#define PER_CPU_BASE_SECTION ".data"
#endif
#endif

in include/asm-generic/percpu.h.

[PATCH] ARM: two possible fixes for the KALLSYMS build problem

[Russell King - ARM Linux]

On Tue, Jun 12, 2012 at 09:02:29AM -0700, David Brown wrote:
> On Tue, Jun 12, 2012 at 03:32:49PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 11, 2012 at 01:37:48PM -0700, David Brown wrote:
> > > On Fri, Jun 01, 2012 at 09:21:59AM -0700, David Brown wrote:
> > > 
> > > > David Brown (1):
> > > >   ARM: Prevent KALLSYM size mismatch on ARM.
> > > > 
> > > >  arch/arm/kernel/vmlinux.lds.S |    3 +++
> > > >  1 file changed, 3 insertions(+)
> > > 
> > > Just wondering if anyone has had a chance to look at either of these,
> > > or try them.  I haven't seen any KALLSYMS mismatch build errors with
> > > either of these patches applied.
> > 
> > I think variant 2 is the better approach out of either as it doesn't add
> > to the size of the resulting kernel image.
> 
> The first variant doesn't ever increase the size of the kernel,
> either.  Variant two actually makes it a little smaller, since a few
> symbols are eliminated.

Ok.

> The second variant is cleaner, though as long as it is safe in every
> configuration.

Well, we don't have any per-CPU data unless we're building for SMP.
See:

#ifndef PER_CPU_BASE_SECTION
#ifdef CONFIG_SMP
#define PER_CPU_BASE_SECTION ".data..percpu"
#else
#define PER_CPU_BASE_SECTION ".data"
#endif
#endif

in include/asm-generic/percpu.h.