2b034922 breaks kexec on ARM

[Lei Wen]

Hi Russel,

On Tue, Nov 8, 2011 at 5:03 AM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Tue, Nov 08, 2011 at 04:55:43AM +0800, Lei Wen wrote:
>> On Tue, Nov 8, 2011 at 3:20 AM, Russell King - ARM Linux
>> <linux at arm.linux.org.uk> wrote:
>> > On Mon, Nov 07, 2011 at 07:14:53PM +0000, Will Deacon wrote:
>> >> On Mon, Nov 07, 2011 at 07:03:15PM +0000, Russell King - ARM Linux wrote:
>> >> > Hmm, having looked at kernel/kexec.c, this patch is clearly wrong.
>> >> > There are two kernels which can be loaded into kexec - a crash kernel
>> >> > and a default kernel.  These are kept entirely separately (and the
>> >> > per-kernel context is stored in 'image').
>> >> >
>> >> > However, this patch makes machine_kexec_prepare() store into some
>> >> > _global_ variables, which means if you've loaded a crash kernel
>> >> > followed by a default kernel, the values for the crash kernel will
>> >> > be corrupted.
>> >> >
>> >> > So yes, I think a revert of this patch is the right thing.
>> >>
>> >> Great. Would you like me to submit the revert to the patch system or are you
>> >> happy just to do it using git?
>> >
>> > I've reverted it myself (see the tip of the fixes branch).  It's slightly
>> > annoying that the branch was only pulled last night.
>> >
>> >> > I think the only valid thing its doing is moving the copy of the
>> >> > reboot code earlier, but I don't see the value of moving just that.
>> >>
>> >> Neither can I. It seems to be happy enough with the patch reverted, so if it
>> >> ain't broke...
>> >
>> > I guess this is where the problems of poor commit descriptions crop up.
>> > We don't know _why_ the original author of the patch wanted to make the
>> > change - we don't know if they were seeing a problem.
>> >
>>
>> Apologize for messing things up...
>> But we have make in-depth testing over that patch already in our local
>> code base...
>> Does this problem happens for the kernel is running on a SMP soc? Our current
>> testing haven't extend to SMP one, but only have test on non-smp.
>
> It doesn't matter - as I've pointed out, the change is _fundamentally_
> wrong because you're storing state in global variables which refers to
> one of two possible images which may be loaded.

I still cannot get understand your point there... Why image->start would refer
to the default kernel? You means kexec load two kernel at the same time?

>
> That can not work properly for every case.
>
> And you haven't explained why you want the change.
The original intension for this posting is for we want to have a chance to
launch the crash kernel, even when default kernel is hanging, which
means it has no chance to trigger the panic path that lead to load the
crash kernel.

Sorry for not including detailed explaination, I would take care of
this for latter posting patch...

Thanks,
Lei