Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations

From: Simon Horman <horms@verge.net.au>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "kexec@lists.infradead.org" <kexec@lists.infradead.org>,
	Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations
Date: Tue, 28 Feb 2012 10:35:27 +0900	[thread overview]
Message-ID: <20120228013526.GA7913@verge.net.au> (raw)
In-Reply-To: <20120227172555.46bffedf.akpm@linux-foundation.org>

On Mon, Feb 27, 2012 at 05:25:55PM -0800, Andrew Morton wrote:
> On Tue, 28 Feb 2012 09:51:30 +0900 Simon Horman <horms@verge.net.au> wrote:
> 
> > On Mon, Feb 27, 2012 at 04:26:31PM -0800, Andrew Morton wrote:
> > > On Tue, 28 Feb 2012 09:19:28 +0900
> > > Simon Horman <horms@verge.net.au> wrote:
> > > 
> > > > > > --- a/kernel/kexec.c
> > > > > > +++ b/kernel/kexec.c
> > > > > > @@ -1462,7 +1462,9 @@ static int __init crash_save_vmcoreinfo_init(void)
> > > > > >  
> > > > > >  	VMCOREINFO_SYMBOL(init_uts_ns);
> > > > > >  	VMCOREINFO_SYMBOL(node_online_map);
> > > > > > +#ifdef CONFIG_MMU
> > > > > >  	VMCOREINFO_SYMBOL(swapper_pg_dir);
> > > > > > +#endif
> > > > > >  	VMCOREINFO_SYMBOL(_stext);
> > > > > >  	VMCOREINFO_SYMBOL(vmlist);
> > > > > 
> > > > > Well, what might be the effects of this patch?  nommu crashfiles will
> > > > > no longer have the swapper_pg_dir string?  What are the chances that
> > > > > someone's (badly written!) downstream tool will crash and burn if this
> > > > > is absent?
> > > > 
> > > > My understanding is that up until this patch creating a dump
> > > > for nonmmu platform wouldn't work.
> > > 
> > > Surprised.  From reading the code I expect it would have emitted 
> > > 
> > > 	SYMBOL(swapper_pg_dir)=0
> > 
> > Hi Andrew,
> > 
> > My understanding from discussion with Will earlier in this thread is that if
> > CONFIG_MMU is not defined then swapper_pg_dir is NULL and the current code
> > doesn't compile.
> > 
> > 	<quote>
> > 	The only case where it's interesting is when you have CONFIG_MMU
> > 	enabled - otherwise it's always NULL. If it's #defined as NULL, the
> > 	current code will fail at build time so simply omitting it from the
> > 	dump seems like the best bet to me (the alternative being to add a
> > 	NULL entry explicitly, but I don't see what the gains us).
> > 	</quote>
> 
> OK.  That's because everything is all screwed up ;)
> 
> swapper_pg_dir is normally an array.  But on nommu it is a pointer. 
> VMCOREINFO_SYMBOL() wants to take its address (unnecessary on an array)
> and this blows up when fed a pointer.
> 
> 
> Still, you didn't answer my question!  What effect will the absence of
> SYMBOL(swapper_pg_dir)= have upon downstream tools?  If "none" then
> sure, let's remove it.  If "explosion" then we should emit a dummy
> SYMBOL(swapper_pg_dir)=0 if CONFIG_NOMMU.

My thought was that the tools wouldn't be used in the CONFIG_NOMMU case (yet).
But I take your point and I think the answer is that the fallout is unknown.
Emitting a dummy value as you suggest seems reasonable.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec