On Wed, Oct 22, 2008 at 04:47:19PM -0700, Jay Lan wrote: > Simon Horman wrote: > > On Tue, Oct 21, 2008 at 09:52:27AM -0700, Jay Lan wrote: > >> Simon Horman wrote: > >> > >> Hi Simon, > >> > >> Just got back from vacation. Sorry for late response. > >> > >>> This bug was discovered by Jay Lan and he also proposed this fix, however > >>> thee is some discussion about what if any related changes should be made at > >>> the same time. > >>> > >>> The bug comes about because the break statment was never executed because > >>> the if clause would bever be true because the if clause will never be true > >>> because & has higher precedence than !=. > >>> > >>> My position on this is that with the if logic fixed, as per this patch, the > >>> break statment and the rest of the while() loop makes sense and should work > >>> as intended. > >>> > >>> As I understand it, Jay's position is that the code should be simplified, > >>> after all it never worked as intended. > >>> > >>> There is a related kernel bug that lead Jay to discover this problem. > >>> The kernel bug has been resolved by Tony Luck and was > >>> included in Linus's tree between 2.6.27-rc8 and 2.6.27-rc9 as > >>> "[IA64] Put the space for cpu0 per-cpu area into .data section". > >>> > >>> Now that the kernel bug is out of the way, I am providing this patch to > >>> continue discussion on what to do on the kexec-tools side of things. I do > >>> not intend to apply this patch until there is some conclusion in the > >>> discussion between Jay and myself. > >> I think this patch is not right for two reasons: > >> 1) The if-statement below has never proved the correctness of > >> its intent because the 'break' statement never got executed > >> due to a logic error. > >> if (loaded_segments[loaded_segments_num].end != > >> (phdr->p_paddr & ~(ELF_PAGE_SIZE-1))) > >> break; > >> 2) With your patch in my testing, the kdump kernel boot hung > >> even earlier in a PAL_CALL that was not returned to the kernel. > >> I understand that my test case was based on a kernel without > >> Tony's latest fix, but that was the only situation we can > >> see the if-statement becomes true. I do not know any other > >> way to make a memory gap happen. However, when it happens, > >> your patch only makes kdump kenrel boot hang earlier. > >> > >> I still root for my patch because the kdump kernel would boot > >> correctly even if a memory gap indeed happened. ;) However, > >> if you do not feel comfortable with my patch, i think the best > >> alternative is to take out the if-statement above completely. > > > > Point taken, just to clarify, this is the patch you would like merged? > > Hi Simon, > > Yes. This is the patch that i would like merged. Thanks. I've applied the change. > > From: Jay Lan <jlan at sgi.com> > > > > IA64: better calculate PT_LOAD segment size > > > > This patch combines consecutive PL_LOAD segments into one. > > The end address of the last PL_LOAD segment, calculated by > > adding p_memsz to p_paddr & rounded up to ELF_PAGE_SIZE, > > will be the end address of this loaded_segments[] entry. > > > > This patch fixes the kdump kernel MCA problem caused by under- > > allocation of memory and a "kdump broken on ALtix 350" problem > > reported by Bernhard Walle. > > > > Simon, this patch replaces my previous patch I submitted on the > > underallocation issue. -- Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en
