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

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

[Will Deacon]

nommu platforms don't have very interesting swapper_pg_dir pointers and
usually just #define them to NULL, meaning that we can't include them in
the vmcoreinfo on the kexec crash path.

This patch only saves the swapper_pg_dir if we have an MMU.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 kernel/kexec.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 7b08867..cb5d13c 100644
--- 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);
 
-- 
1.7.4.1


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

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

[Simon Horman]

On Fri, Feb 24, 2012 at 02:40:47PM +0000, Will Deacon wrote:
> nommu platforms don't have very interesting swapper_pg_dir pointers and
> usually just #define them to NULL, meaning that we can't include them in
> the vmcoreinfo on the kexec crash path.
> 
> This patch only saves the swapper_pg_dir if we have an MMU.

Hi Will,

might this cover cases where swapper_pg_dir is interesting?

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  kernel/kexec.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 7b08867..cb5d13c 100644
> --- 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);
>  
> -- 
> 1.7.4.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

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

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

[Will Deacon]

On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> On Fri, Feb 24, 2012 at 02:40:47PM +0000, Will Deacon wrote:
> > nommu platforms don't have very interesting swapper_pg_dir pointers and
> > usually just #define them to NULL, meaning that we can't include them in
> > the vmcoreinfo on the kexec crash path.
> > 
> > This patch only saves the swapper_pg_dir if we have an MMU.
> 
> Hi Will,

Hi Simon,

> might this cover cases where swapper_pg_dir is interesting?

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).

Cheers,

Will

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

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

[Simon Horman]

On Sun, Feb 26, 2012 at 10:58:42PM +0000, Will Deacon wrote:
> On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> > On Fri, Feb 24, 2012 at 02:40:47PM +0000, Will Deacon wrote:
> > > nommu platforms don't have very interesting swapper_pg_dir pointers and
> > > usually just #define them to NULL, meaning that we can't include them in
> > > the vmcoreinfo on the kexec crash path.
> > > 
> > > This patch only saves the swapper_pg_dir if we have an MMU.
> > 
> > Hi Will,
> 
> Hi Simon,
> 
> > might this cover cases where swapper_pg_dir is interesting?
> 
> 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).

Hi Will,

thanks for the clarification. FWIW I am happy with your patch.

Reviewed-by: Simon Horman <horms@verge.net.au>

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

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

[Will Deacon]

On Mon, Feb 27, 2012 at 12:37:30AM +0000, Simon Horman wrote:
> On Sun, Feb 26, 2012 at 10:58:42PM +0000, Will Deacon wrote:
> > On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> > 
> > > might this cover cases where swapper_pg_dir is interesting?
> > 
> > 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).
> 
> Hi Will,
> 
> thanks for the clarification. FWIW I am happy with your patch.
> 
> Reviewed-by: Simon Horman <horms@verge.net.au>

Cheers, Simon. What's the usual path for generic kexec code getting into
mainline? Do I need to get this queued somewhere?

Will

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

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

[Simon Horman]

On Mon, Feb 27, 2012 at 07:30:54PM +0000, Will Deacon wrote:
> On Mon, Feb 27, 2012 at 12:37:30AM +0000, Simon Horman wrote:
> > On Sun, Feb 26, 2012 at 10:58:42PM +0000, Will Deacon wrote:
> > > On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> > > 
> > > > might this cover cases where swapper_pg_dir is interesting?
> > > 
> > > 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).
> > 
> > Hi Will,
> > 
> > thanks for the clarification. FWIW I am happy with your patch.
> > 
> > Reviewed-by: Simon Horman <horms@verge.net.au>
> 
> Cheers, Simon. What's the usual path for generic kexec code getting into
> mainline? Do I need to get this queued somewhere?

I think that Andrew Morton often picks up these kind of patches.

Andrew, can you pick this up if its reposted with you CCed?
For reference the patch is as follows:


From: Will Deacon <will.deacon@arm.com>

kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations

nommu platforms don't have very interesting swapper_pg_dir pointers and
usually just #define them to NULL, meaning that we can't include them in
the vmcoreinfo on the kexec crash path.

This patch only saves the swapper_pg_dir if we have an MMU.

Signed-off-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Simon Horman <horms@verge.net.au>

---
 kernel/kexec.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 7b08867..cb5d13c 100644
--- 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);
 
-- 
1.7.4.1


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

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

[Andrew Morton]

On Tue, 28 Feb 2012 08:52:35 +0900
Simon Horman <horms@verge.net.au> wrote:

> On Mon, Feb 27, 2012 at 07:30:54PM +0000, Will Deacon wrote:
> > On Mon, Feb 27, 2012 at 12:37:30AM +0000, Simon Horman wrote:
> > > On Sun, Feb 26, 2012 at 10:58:42PM +0000, Will Deacon wrote:
> > > > On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> > > > 
> > > > > might this cover cases where swapper_pg_dir is interesting?
> > > > 
> > > > 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).
> > > 
> > > Hi Will,
> > > 
> > > thanks for the clarification. FWIW I am happy with your patch.
> > > 
> > > Reviewed-by: Simon Horman <horms@verge.net.au>
> > 
> > Cheers, Simon. What's the usual path for generic kexec code getting into
> > mainline? Do I need to get this queued somewhere?
> 
> I think that Andrew Morton often picks up these kind of patches.

I'll take anything, if I have any chance of vaguely understanding it ;)

> Andrew, can you pick this up if its reposted with you CCed?
> For reference the patch is as follows:
> 
> 
> From: Will Deacon <will.deacon@arm.com>
> 
> kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations
> 
> nommu platforms don't have very interesting swapper_pg_dir pointers and
> usually just #define them to NULL, meaning that we can't include them in
> the vmcoreinfo on the kexec crash path.
> 
> This patch only saves the swapper_pg_dir if we have an MMU.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Reviewed-by: Simon Horman <horms@verge.net.au>
> 
> ---
>  kernel/kexec.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 7b08867..cb5d13c 100644
> --- 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?


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

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

[Simon Horman]

On Mon, Feb 27, 2012 at 03:56:43PM -0800, Andrew Morton wrote:
> On Tue, 28 Feb 2012 08:52:35 +0900
> Simon Horman <horms@verge.net.au> wrote:
> 
> > On Mon, Feb 27, 2012 at 07:30:54PM +0000, Will Deacon wrote:
> > > On Mon, Feb 27, 2012 at 12:37:30AM +0000, Simon Horman wrote:
> > > > On Sun, Feb 26, 2012 at 10:58:42PM +0000, Will Deacon wrote:
> > > > > On Sat, Feb 25, 2012 at 02:37:49AM +0000, Simon Horman wrote:
> > > > > 
> > > > > > might this cover cases where swapper_pg_dir is interesting?
> > > > > 
> > > > > 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).
> > > > 
> > > > Hi Will,
> > > > 
> > > > thanks for the clarification. FWIW I am happy with your patch.
> > > > 
> > > > Reviewed-by: Simon Horman <horms@verge.net.au>
> > > 
> > > Cheers, Simon. What's the usual path for generic kexec code getting into
> > > mainline? Do I need to get this queued somewhere?
> > 
> > I think that Andrew Morton often picks up these kind of patches.
> 
> I'll take anything, if I have any chance of vaguely understanding it ;)
> 
> > Andrew, can you pick this up if its reposted with you CCed?
> > For reference the patch is as follows:
> > 
> > 
> > From: Will Deacon <will.deacon@arm.com>
> > 
> > kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations
> > 
> > nommu platforms don't have very interesting swapper_pg_dir pointers and
> > usually just #define them to NULL, meaning that we can't include them in
> > the vmcoreinfo on the kexec crash path.
> > 
> > This patch only saves the swapper_pg_dir if we have an MMU.
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > Reviewed-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> >  kernel/kexec.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > index 7b08867..cb5d13c 100644
> > --- 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.

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

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

[Andrew Morton]

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

?

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

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

[Simon Horman]

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>


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

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

[Andrew Morton]

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.


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

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

[Simon Horman]

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

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

[Will Deacon]

Hi Simon, Andrew,

Sorry for re-joining the thread late, I was sleeping :)

On Tue, Feb 28, 2012 at 01:35:27AM +0000, Simon Horman wrote:
> On Mon, Feb 27, 2012 at 05:25:55PM -0800, Andrew Morton wrote:
> > 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.

Yup. It's a fairly dull pointer at that, which is why I chose to remove it in
the nommu case.

> > 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.

The reason I didn't choose a dummy value to start with is because there are
plenty of other fields in the dump that are predicated on build-time CONFIG
options. Admittedly, some of these appear to be tagged with length
information, but consider these guys:

#ifndef CONFIG_NEED_MULTIPLE_NODES
        VMCOREINFO_SYMBOL(mem_map);
        VMCOREINFO_SYMBOL(contig_page_data);
#endif

Since the VMCOREINFO_SYMBOL macro stringifies the identifier, I guess userspace
has to treat the whole thing as a map, meaning that we *can* safely remove
elements from it.

So that's my speculation. Maybe somebody on the kexec list can confirm?

Cheers,

Will

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

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

[Andrew Morton]

On Tue, 28 Feb 2012 09:40:57 +0000
Will Deacon <will.deacon@arm.com> wrote:

> > > 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.
> 
> The reason I didn't choose a dummy value to start with is because there are
> plenty of other fields in the dump that are predicated on build-time CONFIG
> options. Admittedly, some of these appear to be tagged with length
> information, but consider these guys:
> 
> #ifndef CONFIG_NEED_MULTIPLE_NODES
>         VMCOREINFO_SYMBOL(mem_map);
>         VMCOREINFO_SYMBOL(contig_page_data);
> #endif
> 
> Since the VMCOREINFO_SYMBOL macro stringifies the identifier, I guess userspace
> has to treat the whole thing as a map, meaning that we *can* safely remove
> elements from it.
> 
> So that's my speculation. Maybe somebody on the kexec list can confirm?

Yeah, I do think the chance of breaking anything here is small, and in
the unlikely event that something *does* break then a) it deserved to
break ;) and b) the user of that app will be a developer who can
unbreak it.

Let's run with the patch as-is.

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

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

[Simon Horman]

On Tue, Feb 28, 2012 at 12:45:54PM -0800, Andrew Morton wrote:
> On Tue, 28 Feb 2012 09:40:57 +0000
> Will Deacon <will.deacon@arm.com> wrote:
> 
> > > > 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.
> > 
> > The reason I didn't choose a dummy value to start with is because there are
> > plenty of other fields in the dump that are predicated on build-time CONFIG
> > options. Admittedly, some of these appear to be tagged with length
> > information, but consider these guys:
> > 
> > #ifndef CONFIG_NEED_MULTIPLE_NODES
> >         VMCOREINFO_SYMBOL(mem_map);
> >         VMCOREINFO_SYMBOL(contig_page_data);
> > #endif
> > 
> > Since the VMCOREINFO_SYMBOL macro stringifies the identifier, I guess userspace
> > has to treat the whole thing as a map, meaning that we *can* safely remove
> > elements from it.
> > 
> > So that's my speculation. Maybe somebody on the kexec list can confirm?
> 
> Yeah, I do think the chance of breaking anything here is small, and in
> the unlikely event that something *does* break then a) it deserved to
> break ;) and b) the user of that app will be a developer who can
> unbreak it.
> 
> Let's run with the patch as-is.

Fine by me :)

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