[PATCH] Do not inline putprops function

[Neil Horman]

On Wed, Jun 17, 2009 at 07:56:52PM +0530, M. Mohan Kumar wrote:
> On Wed, Jun 17, 2009 at 10:05:14AM -0400, Neil Horman wrote:
> > On Wed, Jun 17, 2009 at 07:04:35PM +0530, M. Mohan Kumar wrote:
> > > On Wed, Jun 17, 2009 at 09:04:13AM -0400, Neil Horman wrote:
> > > > On Wed, Jun 17, 2009 at 10:26:35PM +1000, Michael Ellerman wrote:
> > > > > 
> > > > > What compiler version are you using? Does the behaviour change if you
> > > > > use a newer/older compiler? It sounds to me like there's some deeper bug
> > > > > and your patch is just papering over it.
> > > > > 
> > > 
> > > I tried with gcc 4.3.2. Let me try with a recent version and update.
> > > 
> > > > Agreed, this doesn't make any sense. Try changing the compiler version to see if
> > > > the problem goes away or stops.  It might also be worthwhile to dump the
> > > > contents of the device tree at the start and end of the kexec process.  If the
> > > > changing of how a function is inlined is causing a hang, its likely changing how
> > > > the putprops function is writing information to the device tree.  Understanding
> > > > what that change is will likely provide clues to how the code has changed.
> > > 
> > > Neil, there was no code change in fs2dt.c
> > > 
> > Sure there was, by modifying the code to tell it to not inline the putprops
> > function, you changed how the complier optimizes that code block.  There may not
> > be any source level code change, but the assembly is certainly different, and it
> > must be so in a way thats causing a hang.  The putpops function changes the
> > contents of the ppc64 device-tree, so if this is a compiler bug, and its causing
> > a hang, I imagine we'll see some manifestation in a change in the device tree
> > contents.
> 
> As I mentioned in the patch description, when I retained the variable dt_len
> and dt_len = dt assignment, kexec/kdump kernel would work. But even if I
> comment the "dt_len = dt" assignment kernel would hang. If you need I can
Yes, and as Michael has noted, that is likey an indication of a compiler bug.
You shouldn't get differing behavior in a kdump kernel by removing an unused
variable in kexec.  Which is why I'm unconvinced this patch is really fixing
anything, but rather just avoiding the real problem.

> send objdump of fs2dt.o with and without this assignment.
> 
That would be a fine thing to do, and I'd be happy to compare them.  My though
regarding the comparison of the device tree on a good and bad run was meant to
expidite what change in the assembly we'd be looking for.  If its the kdump
kernel boot thats hanging, Its likely hanging on something in the device tree,
as thats whats being manipulated by this code.  So I figure that understanding
whats changed there will point us toward what change in the assembly might be
responsible for the hang.  The assmebly's going to be signficantly different
(lots of optimization might be lost from no longer inlining a function), so
anything that helps us narrow down whats changed will be good
Neil

> Regards,
> M. Mohan Kumar.