* [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations @ 2012-02-24 14:40 Will Deacon 2012-02-25 2:37 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Will Deacon @ 2012-02-24 14:40 UTC (permalink / raw) To: kexec; +Cc: 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-24 14:40 [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations Will Deacon @ 2012-02-25 2:37 ` Simon Horman 2012-02-26 22:58 ` Will Deacon 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-25 2:37 UTC (permalink / raw) To: Will Deacon; +Cc: kexec 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-25 2:37 ` Simon Horman @ 2012-02-26 22:58 ` Will Deacon 2012-02-27 0:37 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Will Deacon @ 2012-02-26 22:58 UTC (permalink / raw) To: Simon Horman; +Cc: kexec@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-26 22:58 ` Will Deacon @ 2012-02-27 0:37 ` Simon Horman 2012-02-27 19:30 ` Will Deacon 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-27 0:37 UTC (permalink / raw) To: Will Deacon; +Cc: kexec@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-27 0:37 ` Simon Horman @ 2012-02-27 19:30 ` Will Deacon 2012-02-27 23:52 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Will Deacon @ 2012-02-27 19:30 UTC (permalink / raw) To: Simon Horman; +Cc: kexec@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-27 19:30 ` Will Deacon @ 2012-02-27 23:52 ` Simon Horman 2012-02-27 23:56 ` Andrew Morton 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-27 23:52 UTC (permalink / raw) To: Will Deacon; +Cc: Andrew Morton, kexec@lists.infradead.org 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-27 23:52 ` Simon Horman @ 2012-02-27 23:56 ` Andrew Morton 2012-02-28 0:19 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2012-02-27 23:56 UTC (permalink / raw) To: Simon Horman; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-27 23:56 ` Andrew Morton @ 2012-02-28 0:19 ` Simon Horman 2012-02-28 0:26 ` Andrew Morton 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-28 0:19 UTC (permalink / raw) To: Andrew Morton; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 0:19 ` Simon Horman @ 2012-02-28 0:26 ` Andrew Morton 2012-02-28 0:51 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2012-02-28 0:26 UTC (permalink / raw) To: Simon Horman; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 0:26 ` Andrew Morton @ 2012-02-28 0:51 ` Simon Horman 2012-02-28 1:25 ` Andrew Morton 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-28 0:51 UTC (permalink / raw) To: Andrew Morton; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 0:51 ` Simon Horman @ 2012-02-28 1:25 ` Andrew Morton 2012-02-28 1:35 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2012-02-28 1:25 UTC (permalink / raw) To: Simon Horman; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 1:25 ` Andrew Morton @ 2012-02-28 1:35 ` Simon Horman 2012-02-28 9:40 ` Will Deacon 0 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2012-02-28 1:35 UTC (permalink / raw) To: Andrew Morton; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 1:35 ` Simon Horman @ 2012-02-28 9:40 ` Will Deacon 2012-02-28 20:45 ` Andrew Morton 0 siblings, 1 reply; 15+ messages in thread From: Will Deacon @ 2012-02-28 9:40 UTC (permalink / raw) To: Simon Horman; +Cc: Andrew Morton, kexec@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 9:40 ` Will Deacon @ 2012-02-28 20:45 ` Andrew Morton 2012-02-29 1:32 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2012-02-28 20:45 UTC (permalink / raw) To: Will Deacon; +Cc: Simon Horman, kexec@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations 2012-02-28 20:45 ` Andrew Morton @ 2012-02-29 1:32 ` Simon Horman 0 siblings, 0 replies; 15+ messages in thread From: Simon Horman @ 2012-02-29 1:32 UTC (permalink / raw) To: Andrew Morton; +Cc: kexec@lists.infradead.org, Will Deacon 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-02-29 1:33 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-24 14:40 [PATCH] kexec: crash: don't save swapper_pg_dir for !CONFIG_MMU configurations Will Deacon 2012-02-25 2:37 ` Simon Horman 2012-02-26 22:58 ` Will Deacon 2012-02-27 0:37 ` Simon Horman 2012-02-27 19:30 ` Will Deacon 2012-02-27 23:52 ` Simon Horman 2012-02-27 23:56 ` Andrew Morton 2012-02-28 0:19 ` Simon Horman 2012-02-28 0:26 ` Andrew Morton 2012-02-28 0:51 ` Simon Horman 2012-02-28 1:25 ` Andrew Morton 2012-02-28 1:35 ` Simon Horman 2012-02-28 9:40 ` Will Deacon 2012-02-28 20:45 ` Andrew Morton 2012-02-29 1:32 ` Simon Horman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.