Commit bba0859a99 (arm: versatile: don't mark pen as __INIT) introduced the following section mismatch warnings:
WARNING: vmlinux.o(.text+0x18208): Section mismatch in reference from the variable pen to the function .cpuinit.text:secondary_startup() WARNING: vmlinux.o(.text+0x18210): Section mismatch in reference from the variable pen to the variable .cpuinit.data:pen_release
The first is handled by removing __cpuinitdata from pen_release. This also fixes and potential bug because the issue commit bba0859a99 was aimed at fixing meant a CPU not known to the kernel could be spinning forever in versatile_secondary_startup and polling this pen_release variable, so it is important its memory isn't discarded and reused after boot.
The second section mismatch warning is removed by taking __CPUINIT away from before secondary_startup.
Signed-off-by: Jon Medhurst tixy@linaro.org ---
This patch is aimed at the Linaro Stable Kernel. Do we need some standard way of annotating patches for that?
Note, this issue does not affect mainline Linux or stable kernel trees because the upstream version of the problematic patch (commit 8121cf312a1) first appears in Linux 3.11 which also removed all cpuinit annotations in the kernel.
arch/arm/kernel/head.S | 1 - arch/arm/kernel/smp.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 8bac553..2725c87 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -342,7 +342,6 @@ __turn_mmu_on_loc: .long __turn_mmu_on_end
#if defined(CONFIG_SMP) - __CPUINIT ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 84d55da..a666911 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -57,7 +57,7 @@ struct secondary_data secondary_data; * control for which core is the next to come out of the secondary * boot "holding pen" */ -volatile int __cpuinitdata pen_release = -1; +volatile int pen_release = -1;
enum ipi_msg_type { IPI_WAKEUP,
+1 looks good to me....
-----Original Message----- From: Jon Medhurst (Tixy) [mailto:tixy@linaro.org] Sent: 06 September 2013 15:10 To: Mark Brown Cc: Mark Hambleton; James King; Alex Shi; Linaro Kernel Subject: [PATCH] ARM: kernel: Fix section mismatches caused by commit bba0859a99
Commit bba0859a99 (arm: versatile: don't mark pen as __INIT) introduced the following section mismatch warnings:
WARNING: vmlinux.o(.text+0x18208): Section mismatch in reference from the variable pen to the function .cpuinit.text:secondary_startup() WARNING: vmlinux.o(.text+0x18210): Section mismatch in reference from the variable pen to the variable .cpuinit.data:pen_release
The first is handled by removing __cpuinitdata from pen_release. This also fixes and potential bug because the issue commit bba0859a99 was aimed at fixing meant a CPU not known to the kernel could be spinning forever in versatile_secondary_startup and polling this pen_release variable, so it is important its memory isn't discarded and reused after boot.
The second section mismatch warning is removed by taking __CPUINIT away from before secondary_startup.
Signed-off-by: Jon Medhurst tixy@linaro.org ---
This patch is aimed at the Linaro Stable Kernel. Do we need some standard way of annotating patches for that?
Note, this issue does not affect mainline Linux or stable kernel trees because the upstream version of the problematic patch (commit 8121cf312a1) first appears in Linux 3.11 which also removed all cpuinit annotations in the kernel.
arch/arm/kernel/head.S | 1 - arch/arm/kernel/smp.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 8bac553..2725c87 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -342,7 +342,6 @@ __turn_mmu_on_loc: .long __turn_mmu_on_end
#if defined(CONFIG_SMP) - __CPUINIT ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 84d55da..a666911 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -57,7 +57,7 @@ struct secondary_data secondary_data; * control for which core is the next to come out of the secondary * boot "holding pen" */ -volatile int __cpuinitdata pen_release = -1; +volatile int pen_release = -1;
enum ipi_msg_type { IPI_WAKEUP, -- 1.7.10.4
Tixy, Could you like to send the bad config to reproduce the fixing issue?
On 09/06/2013 11:25 PM, Mark Hambleton wrote:
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 8bac553..2725c87 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -342,7 +342,6 @@ __turn_mmu_on_loc: .long __turn_mmu_on_end #if defined(CONFIG_SMP)
- __CPUINIT ENTRY(secondary_startup)
I just has few concern of the solution of upstream kernel. the commit 2449189bb7c73b5fe55a18bc0d289e39bdcd4998 in Linus' tree did similar fixing.
#if defined(CONFIG_SMP) + .text ENTRY(secondary_startup) /*
/* * Common entry point for secondary CPUs. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 84d55da..a666911 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -57,7 +57,7 @@ struct secondary_data secondary_data;
- control for which core is the next to come out of the secondary
- boot "holding pen"
*/ -volatile int __cpuinitdata pen_release = -1; +volatile int pen_release = -1;
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus' tree too. I saw Greg also picked up the commit.
Tixy, I appreciate your great effort on this bug. But as my understanding, if we pick up this patch, linaro stable tree will be ugly when merge the Greg's stable tree again.
Mark, what's your comments of this?
Regards Alex
enum ipi_msg_type { IPI_WAKEUP, -- 1.7.10.4
On 09/10/2013 01:58 PM, Alex Shi wrote:
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus' tree too. I saw Greg also picked up the commit.
Tixy, I appreciate your great effort on this bug. But as my understanding, if we pick up this patch, linaro stable tree will be ugly when merge the Greg's stable tree again.
What I want to say is, if the upstream kernel fixed this bug, we don't need to pick up another patch for this.
Regards!
Mark, what's your comments of this?
On Tue, 2013-09-10 at 13:58 +0800, Alex Shi wrote:
Could you like to send the bad config to reproduce the fixing issue?
This occurs with the Versatile Express config used in Linaro, i.e. the output created by...
ARCH=arm scripts/kconfig/merge_config.sh \ linaro/configs/linaro-base.conf \ linaro/configs/distribution.conf \ linaro/configs/big-LITTLE-MP.conf \ linaro/configs/big-LITTLE-IKS.conf \ linaro/configs/vexpress.conf
Though to get the section missmatch from secondary_startup you will also need to disable CONFIG_THUMB2_KERNEL and section mismatch checking doesn't work full for Thumb kernels.
Oh, and you will need CONFIG_DEBUG_SECTION_MISMATCH=y as well. I always have that on my commandline when building kernels.
On 09/06/2013 11:25 PM, Mark Hambleton wrote:
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 8bac553..2725c87 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -342,7 +342,6 @@ __turn_mmu_on_loc: .long __turn_mmu_on_end #if defined(CONFIG_SMP)
- __CPUINIT ENTRY(secondary_startup)
I just has few concern of the solution of upstream kernel. the commit 2449189bb7c73b5fe55a18bc0d289e39bdcd4998 in Linus' tree did similar fixing.
That fixes the other commit you mention below and doesn't work properly without it...
#if defined(CONFIG_SMP)
ENTRY(secondary_startup) /*.text
/* * Common entry point for secondary CPUs. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 84d55da..a666911 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -57,7 +57,7 @@ struct secondary_data secondary_data;
- control for which core is the next to come out of the secondary
- boot "holding pen"
*/ -volatile int __cpuinitdata pen_release = -1; +volatile int pen_release = -1;
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus' tree too. I saw Greg also picked up the commit.
That is the ARM part of the whole cpuinit feature removal, so if that's being backported to the 3.10 stable kernel then none of my patch is needed and we can just ignore it and get on with other things :-)
On 09/10/2013 04:39 PM, Jon Medhurst (Tixy) wrote:
On Tue, 2013-09-10 at 13:58 +0800, Alex Shi wrote:
Could you like to send the bad config to reproduce the fixing issue?
This occurs with the Versatile Express config used in Linaro, i.e. the output created by...
ARCH=arm scripts/kconfig/merge_config.sh \ linaro/configs/linaro-base.conf \ linaro/configs/distribution.conf \ linaro/configs/big-LITTLE-MP.conf \ linaro/configs/big-LITTLE-IKS.conf \ linaro/configs/vexpress.conf
Though to get the section missmatch from secondary_startup you will also need to disable CONFIG_THUMB2_KERNEL and section mismatch checking doesn't work full for Thumb kernels.
Oh, and you will need CONFIG_DEBUG_SECTION_MISMATCH=y as well. I always have that on my commandline when building kernels.
thanks!
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus' tree too. I saw Greg also picked up the commit.
That is the ARM part of the whole cpuinit feature removal, so if that's being backported to the 3.10 stable kernel then none of my patch is needed and we can just ignore it and get on with other things :-)
Greg doesn't backport this commit to 3.10 now, but I saw it is included in 3.11 stable branch. What our policy for such issues, Mark?
On 10 September 2013 10:05, Alex Shi alex.shi@linaro.org wrote:
On 09/10/2013 04:39 PM, Jon Medhurst (Tixy) wrote:
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus' tree too. I saw Greg also picked up the commit.
That is the ARM part of the whole cpuinit feature removal, so if that's being backported to the 3.10 stable kernel then none of my patch is needed and we can just ignore it and get on with other things :-)
Greg doesn't backport this commit to 3.10 now, but I saw it is included in 3.11 stable branch. What our policy for such issues, Mark?
In this case the ideal thing would be to cherry-pick it back and then submit it to Greg, picking it up once Greg has applied it - I strongly suspect that the reason he didn't pull it back was that it didn't apply cleanly due to other changes. However just applying Tixy's change and noting that it corresponds to the upstream commit is OK for resolving this issue. We can also do both, things should merge down cleanly.
On 10 September 2013 17:48, Mark Brown broonie@linaro.org wrote:
On 10 September 2013 10:05, Alex Shi alex.shi@linaro.org wrote:
On 09/10/2013 04:39 PM, Jon Medhurst (Tixy) wrote:
and commit 8bd26e3a7e49af2697449bbc, includes this change on Linus'
tree
too. I saw Greg also picked up the commit.
That is the ARM part of the whole cpuinit feature removal, so if that's being backported to the 3.10 stable kernel then none of my patch is needed and we can just ignore it and get on with other things :-)
Greg doesn't backport this commit to 3.10 now, but I saw it is included in 3.11 stable branch. What our policy for such issues, Mark?
In this case the ideal thing would be to cherry-pick it back and then submit it to Greg, picking it up once Greg has applied it - I strongly suspect that the reason he didn't pull it back was that it didn't apply cleanly due to other changes. However just applying Tixy's change and noting that it corresponds to the upstream commit is OK for resolving this issue. We can also do both, things should merge down cleanly.
Thanks a lot, Mark. Will try to cherry-pick related commit on greg's tree first tomorrow. and talk this issue with Greg. If the reason is like your guess. :) we can do the second solution.
Thanks again for your comments!
On 10 September 2013 16:26, Alex Shi alex.shi@linaro.org wrote:
Thanks a lot, Mark. Will try to cherry-pick related commit on greg's tree first tomorrow. and talk this issue with Greg. If the reason is like your guess. :) we can do the second solution.
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
On 09/11/2013 12:04 AM, Mark Brown wrote:
On 10 September 2013 16:26, Alex Shi <alex.shi@linaro.org mailto:alex.shi@linaro.org> wrote:
Thanks a lot, Mark. Will try to cherry-pick related commit on greg's tree first tomorrow. and talk this issue with Greg. If the reason is like your guess. :) we can do the second solution.
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
OK. got it.
On 09/11/2013 12:04 AM, Mark Brown wrote:
On 10 September 2013 16:26, Alex Shi <alex.shi@linaro.org mailto:alex.shi@linaro.org> wrote:
Thanks a lot, Mark. Will try to cherry-pick related commit on greg's tree first tomorrow. and talk this issue with Greg. If the reason is like your guess. :) we can do the second solution.
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
Hi Mark, As your estimation, related upstream commits have much problems to apply on Greg's 3.10 stable kernel.
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
---
From 2ba254d32cee6c1df4636c6173c924d16c40948c Mon Sep 17 00:00:00 2001
From: "Jon Medhurst (Tixy)" tixy@linaro.org Date: Tue, 10 Sep 2013 12:01:44 +0800 Subject: [PATCH] ARM: kernel: Fix section mismatches caused by commit bba0859a99
Commit bba0859a99 (arm: versatile: don't mark pen as __INIT) introduced the following section mismatch warnings:
WARNING: vmlinux.o(.text+0x18208): Section mismatch in reference from the variable pen to the function .cpuinit.text:secondary_startup() WARNING: vmlinux.o(.text+0x18210): Section mismatch in reference from the variable pen to the variable .cpuinit.data:pen_release
The first is handled by removing __cpuinitdata from pen_release. This also fixes and potential bug because the issue commit bba0859a99 was aimed at fixing meant a CPU not known to the kernel could be spinning forever in versatile_secondary_startup and polling this pen_release variable, so it is important its memory isn't discarded and reused after boot.
The second section mismatch warning is removed by taking __CPUINIT away from before secondary_startup.
Signed-off-by: Jon Medhurst tixy@linaro.org Signed-off-by: Alex Shi alex.shi@linaro.org --- arch/arm/kernel/head.S | 1 - arch/arm/kernel/smp.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 8bac553..2725c87 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -342,7 +342,6 @@ __turn_mmu_on_loc: .long __turn_mmu_on_end
#if defined(CONFIG_SMP) - __CPUINIT ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index dc05ca7..46e8ab3 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -57,7 +57,7 @@ struct secondary_data secondary_data; * control for which core is the next to come out of the secondary * boot "holding pen" */ -volatile int __cpuinitdata pen_release = -1; +volatile int pen_release = -1;
enum ipi_msg_type { IPI_WAKEUP,
On 09/11/2013 02:11 PM, Alex Shi wrote:
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
Hi Mark, As your estimation, related upstream commits have much problems to apply on Greg's 3.10 stable kernel.
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
BTW, after pick up a new patch, how to trigger kernel build/boot and lava system to test the kernel tree?
On 11 September 2013 09:41, Alex Shi alex.shi@linaro.org wrote:
On 09/11/2013 02:11 PM, Alex Shi wrote:
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
Hi Mark, As your estimation, related upstream commits have much problems to apply on Greg's 3.10 stable kernel.
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
BTW, after pick up a new patch, how to trigger kernel build/boot and lava system to test the kernel tree?
the CI job is pulling from linux-linaro-stable tree: https://git.linaro.org/gitweb?p=kernel/linux-linaro-stable.git%3Ba=summary
the patch hasn't been pushed yet.
to trigger a build: go to https://ci.linaro.org/jenkins/job/linux-linaro-stable-lsk/ click build now button (you should be authenticated)
On 09/11/2013 03:30 PM, Fathi Boudra wrote:
On 11 September 2013 09:41, Alex Shi alex.shi@linaro.org wrote:
On 09/11/2013 02:11 PM, Alex Shi wrote:
OK. Please keep me in the CCs if you mail Greg, and it probably makes sense to loop in the original author too.
Hi Mark, As your estimation, related upstream commits have much problems to apply on Greg's 3.10 stable kernel.
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
BTW, after pick up a new patch, how to trigger kernel build/boot and lava system to test the kernel tree?
the CI job is pulling from linux-linaro-stable tree: https://git.linaro.org/gitweb?p=kernel/linux-linaro-stable.git%3Ba=summary
the patch hasn't been pushed yet.
Yes, guess I need to give my ssh public key to admin of git.linaro.org for push permission, right?
to trigger a build: go to https://ci.linaro.org/jenkins/job/linux-linaro-stable-lsk/ click build now button (you should be authenticated)
Thanks for the info. CI is a great project. It would be better if it can do bisection for failures. :)
On 11 September 2013 08:30, Fathi Boudra fathi.boudra@linaro.org wrote:
BTW, after pick up a new patch, how to trigger kernel build/boot and lava system to test the kernel tree?
the CI job is pulling from linux-linaro-stable tree: https://git.linaro.org/gitweb?p=kernel/linux-linaro-stable.git%3Ba=summary
the patch hasn't been pushed yet.
to trigger a build: go to https://ci.linaro.org/jenkins/job/linux-linaro-stable-lsk/ click build now button (you should be authenticated)
Or the other option is just to wait for the daily build to pick things up.
On 11 September 2013 07:11, Alex Shi alex.shi@linaro.org wrote:
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested
compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
Either works, though you should apply the patch on one of the topic branches then merge that into the LSK branches - probably the warnings branch is best here.
Please push to linux-linaro-lsk and linux-linaro-lsk-android - those are currently the master branches. The changes on linux-linaro-lsk should be merged into the -android branch.
On Wed, 2013-09-11 at 10:51 +0100, Mark Brown wrote:
On 11 September 2013 07:11, Alex Shi alex.shi@linaro.org wrote:
So, I apply Tixy's patch on lsk-v3.10 branch of lsk. and tested compilation. That's fine. Is there some way to let me push the change to lsk. or let me put the change on a git.linaro.org and then You pull my tree for this change?
Either works, though you should apply the patch on one of the topic branches then merge that into the LSK branches - probably the warnings branch is best here.
Or the topic branch with the patch which introduces the warning? I.e. v3.10/topic/tc2 which has bba0859a996589e9673717864d104a7603687941
It's Mark's call though.
On 11 September 2013 11:07, Jon Medhurst (Tixy) tixy@linaro.org wrote:
On Wed, 2013-09-11 at 10:51 +0100, Mark Brown wrote:
Either works, though you should apply the patch on one of the topic branches then merge that into the LSK branches - probably the warnings branch is best here.
Or the topic branch with the patch which introduces the warning? I.e. v3.10/topic/tc2 which has bba0859a996589e9673717864d104a7603687941
It's Mark's call though.
Yes, if it actually depends on the TC2 changes it'll need to go on that branch.
On 09/11/2013 06:09 PM, Mark Brown wrote:
On 11 September 2013 11:07, Jon Medhurst (Tixy) <tixy@linaro.org mailto:tixy@linaro.org> wrote:
On Wed, 2013-09-11 at 10:51 +0100, Mark Brown wrote: > Either works, though you should apply the patch on one of the topic > branches then merge that into the LSK branches - probably the warnings > branch is best here. Or the topic branch with the patch which introduces the warning? I.e. v3.10/topic/tc2 which has bba0859a996589e9673717864d104a7603687941 It's Mark's call though.
Yes, if it actually depends on the TC2 changes it'll need to go on that branch.
Sure. will add to that branch and wait for testing results.
On 09/11/2013 06:09 PM, Mark Brown wrote:
On 11 September 2013 11:07, Jon Medhurst (Tixy) <tixy@linaro.org mailto:tixy@linaro.org> wrote:
On Wed, 2013-09-11 at 10:51 +0100, Mark Brown wrote: > Either works, though you should apply the patch on one of the topic > branches then merge that into the LSK branches - probably the warnings > branch is best here. Or the topic branch with the patch which introduces the warning? I.e. v3.10/topic/tc2 which has bba0859a996589e9673717864d104a7603687941 It's Mark's call though.
Yes, if it actually depends on the TC2 changes it'll need to go on that branch.
Are these topic/xxx branch used for testing purpose for formal lsk? Actually, the tc2 branch has much compilation bugs with lsk kernel config. So, could you push a new topic branch for this patch testing purpose? or forcefully covered the warning branch with lsk + this patch?
On 11 September 2013 15:09, Alex Shi alex.shi@linaro.org wrote:
Are these topic/xxx branch used for testing purpose for formal lsk? Actually, the tc2 branch has much compilation bugs with lsk kernel config. So, could you push a new topic branch for this patch testing purpose? or forcefully covered the warning branch with lsk + this patch?
They aren't individually tested, no - it's only the merge down that gets tested, partly for resourcing reasons. The topic branches are more for understanding the code than anything else.
linaro-kernel@lists.linaro.org