| Message ID | 20130731055807.14054.70106.sendpatchset@w520 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 31, 2013 at 02:58:07PM +0900, Magnus Damm wrote: > From: Magnus Damm <damm@opensource.se> > > This fix for the ARM architecture will include stext > in the memblock_reserve() call to make sure that the > following symbols are not overwritten (from System.map): > > c0008000 T _text > c0008000 T stext > ... > c0008138 T secondary_startup > ... > c0009000 T _stext > > With this patch applied CPU Hotplug starts working > again. Without this patch code in secondary_startup > never gets reached as expected. > > This issue started triggering on kernels later than > v3.10 - perhaps a side effect of the CPU Hotplug init > section rework - so this is a fix for v3.11-rc. This is not the right fix. The commit you previously pointed out (removal of CPU hotplug) probably means there's a missing annotation somewhere in the code. And yes - the __CPUINIT just before ENTRY(secondary_startup) was removed rather than being replaced with a .text (in both head.S and head-nommu.S). Same goes for ENTRY(lookup_processor_type) in head-common.S. All these need to be replaced with .text to stop them falling into the head section.
On Wed, Jul 31, 2013 at 6:43 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Jul 31, 2013 at 02:58:07PM +0900, Magnus Damm wrote: >> From: Magnus Damm <damm@opensource.se> >> >> This fix for the ARM architecture will include stext >> in the memblock_reserve() call to make sure that the >> following symbols are not overwritten (from System.map): >> >> c0008000 T _text >> c0008000 T stext >> ... >> c0008138 T secondary_startup >> ... >> c0009000 T _stext >> >> With this patch applied CPU Hotplug starts working >> again. Without this patch code in secondary_startup >> never gets reached as expected. >> >> This issue started triggering on kernels later than >> v3.10 - perhaps a side effect of the CPU Hotplug init >> section rework - so this is a fix for v3.11-rc. > > This is not the right fix. The commit you previously pointed out (removal > of CPU hotplug) probably means there's a missing annotation somewhere in > the code. > > And yes - the __CPUINIT just before ENTRY(secondary_startup) was removed > rather than being replaced with a .text (in both head.S and head-nommu.S). > Same goes for ENTRY(lookup_processor_type) in head-common.S. All these > need to be replaced with .text to stop them falling into the head section. Thanks for your guidance, Russell. The right way to fix this now seems pretty clear to me. Are you aware of any existing fix for this or anyone working on solving this issue? If not then I don't mind stepping up. I can however think of quite a few other things to do if this has been solved already though, so please let me know if you think I should focus on fixing this. Cheers, / magnus
On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: > Are you aware of any existing fix for this or anyone working on > solving this issue? If not then I don't mind stepping up. I can > however think of quite a few other things to do if this has been > solved already though, so please let me know if you think I should > focus on fixing this. I committed a fix for it this morning, so it should be in linux-next by tomorrow.
On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote: > On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: > > Are you aware of any existing fix for this or anyone working on > > solving this issue? If not then I don't mind stepping up. I can > > however think of quite a few other things to do if this has been > > solved already though, so please let me know if you think I should > > focus on fixing this. > > I committed a fix for it this morning, so it should be in linux-next by > tomorrow. Hi Russell, Magnus has some outstanding patches for Renesas SoC boards which I believe depend on this problem being resolved. Could you indicate when you expect this patch to be in a stable branch which I could use as a base for a branch that will subsequently be pulled into arm-soc. Thanks.
On 13-08-01 03:34 AM, Simon Horman wrote: > On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote: >> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: >>> Are you aware of any existing fix for this or anyone working on >>> solving this issue? If not then I don't mind stepping up. I can >>> however think of quite a few other things to do if this has been >>> solved already though, so please let me know if you think I should >>> focus on fixing this. >> >> I committed a fix for it this morning, so it should be in linux-next by >> tomorrow. > > Hi Russell, > > Magnus has some outstanding patches for Renesas SoC boards which I believe > depend on this problem being resolved. Could you indicate when you expect > this patch to be in a stable branch which I could use as a base for a > branch that will subsequently be pulled into arm-soc. It will never be in a stable release, since the cpuinit removal was added in the 3.11-rc1 merge window content, and the fix will be in before 3.11 final hits the streets. If you want stable stuff, you should be using v3.10 at this point in time. Paul. -- > > Thanks. >
On Thu, Aug 01, 2013 at 08:58:30AM -0400, Paul Gortmaker wrote: > On 13-08-01 03:34 AM, Simon Horman wrote: > > On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote: > >> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: > >>> Are you aware of any existing fix for this or anyone working on > >>> solving this issue? If not then I don't mind stepping up. I can > >>> however think of quite a few other things to do if this has been > >>> solved already though, so please let me know if you think I should > >>> focus on fixing this. > >> > >> I committed a fix for it this morning, so it should be in linux-next by > >> tomorrow. > > > > Hi Russell, > > > > Magnus has some outstanding patches for Renesas SoC boards which I believe > > depend on this problem being resolved. Could you indicate when you expect > > this patch to be in a stable branch which I could use as a base for a > > branch that will subsequently be pulled into arm-soc. > > It will never be in a stable release, since the cpuinit removal was > added in the 3.11-rc1 merge window content, and the fix will be in > before 3.11 final hits the streets. If you want stable stuff, you > should be using v3.10 at this point in time. Sorry for my loose use of the word stable. I meant a branch that will be incorporated into a future mainline release. From your response above I assume that I can use v3.11-rcX for some values of X. Can I confirm that the commit I should be looking for is cd717a2a3db96af1e308d5bdb76c9d5f8154b4bd ("ARM: Add .text annotations where required after __CPUINIT removal")?
On Thu, Aug 1, 2013 at 4:39 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: >> Are you aware of any existing fix for this or anyone working on >> solving this issue? If not then I don't mind stepping up. I can >> however think of quite a few other things to do if this has been >> solved already though, so please let me know if you think I should >> focus on fixing this. > > I committed a fix for it this morning, so it should be in linux-next by > tomorrow. Thanks for writing a fix, Russell. I cherry picked the following commit from linux-arm: cd717a2 ARM: Add .text annotations where required after __CPUINIT removal Applying it on top of kernel.org renesas git tag renesas-devel-20130801v2 (v3.11-rc3 + v3.12 material) solves the problem for me. I just tested this on KZM9G hardware without and with the patch, so I can confirm that CPU Hotplug is now working again. Tested-by: Magnus Damm <damm@opensource.se> Cheers, / magnus
On Fri, Aug 02, 2013 at 12:24:17PM +0900, Magnus Damm wrote: > On Thu, Aug 1, 2013 at 4:39 AM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote: > >> Are you aware of any existing fix for this or anyone working on > >> solving this issue? If not then I don't mind stepping up. I can > >> however think of quite a few other things to do if this has been > >> solved already though, so please let me know if you think I should > >> focus on fixing this. > > > > I committed a fix for it this morning, so it should be in linux-next by > > tomorrow. > > Thanks for writing a fix, Russell. I cherry picked the following > commit from linux-arm: > > cd717a2 ARM: Add .text annotations where required after __CPUINIT removal > > Applying it on top of kernel.org renesas git tag > renesas-devel-20130801v2 (v3.11-rc3 + v3.12 material) solves the > problem for me. I just tested this on KZM9G hardware without and with > the patch, so I can confirm that CPU Hotplug is now working again. > > Tested-by: Magnus Damm <damm@opensource.se> It should've ended up in Linus' tree last night, but from the looks of how quiet things seem to be, I guess everyone's away on August vacations at the moment.
--- 0001/arch/arm/mm/init.c +++ work/arch/arm/mm/init.c 2013-07-31 14:17:49.000000000 +0900 @@ -346,7 +346,7 @@ void __init arm_memblock_init(struct mem #ifdef CONFIG_XIP_KERNEL memblock_reserve(__pa(_sdata), _end - _sdata); #else - memblock_reserve(__pa(_stext), _end - _stext); + memblock_reserve(__pa(_text), _end - _text); #endif #ifdef CONFIG_BLK_DEV_INITRD if (phys_initrd_size &&