All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Petr Tesarik <ptesarik@suse.cz>
Cc: kexec@lists.infradead.org, Eric Biederman <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Put each per-cpu kdump ELF notes into a single page
Date: Thu, 11 Sep 2014 16:01:10 -0400	[thread overview]
Message-ID: <20140911200110.GA24699@redhat.com> (raw)
In-Reply-To: <20140905183314.23690175@hananiah.suse.cz>

On Fri, Sep 05, 2014 at 06:33:14PM +0200, Petr Tesarik wrote:
> On architectures that use percpu-vm, the percpu region is not guaranteed
> to be contiguous in physical space.

Petr,

Which are those arches?

> However, fs/proc/vmcore.c expects
> all ELF notes to be contiguous. If the ELF note happens to occupy
> two non-adjacent physical pages, part of the note may be read from an
> incorrect memory location by the kdump kernel, resulting in failure to
> initialize /proc/vmcore (if the content of the following physical page,
> incorrectly interpreted as an ELF note specifies a large number), wrong
> register values or other apparent random memory corruption.
> 
> There is currently no mechanism to pass the virtual-to-physical mapping
> of the percpu allocation to the kdump kernel. So, instead, I'm changing
> the alignment of the ELF note buffer. Since sizeof(note_buf_t) is less
> than PAGE_SIZE, aligning the buffer to the nearest higher power of 2
> is enough to make sure that the buffer cannot cross a page boundary,
> effectively ensuring that the whole buffer is contiguous in physical
> space.
> 
> Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
> ---
>  kernel/kexec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 2bee072..cdab59d 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -1610,7 +1610,8 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
>  static int __init crash_notes_memory_init(void)
>  {
>  	/* Allocate memory for saving cpu registers. */
> -	crash_notes = alloc_percpu(note_buf_t);
> +	crash_notes = __alloc_percpu(sizeof(note_buf_t),
> +				     roundup_pow_of_two(sizeof(note_buf_t)));

I think some of the changelog should show up here as comment in short
form. I don't think it is obvious that why we are using __alloc_percpu()
and why aligning to nearst higher power of 2 is needed here. Please also
mention here which arches run into issues.

Thanks
Vivek

>  	if (!crash_notes) {
>  		pr_warn("Kexec: Memory allocation for saving cpu register states failed\n");
>  		return -ENOMEM;
> -- 
> 1.8.4.5
> 
> _______________________________________________
> 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

WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Petr Tesarik <ptesarik@suse.cz>
Cc: Eric Biederman <ebiederm@xmission.com>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Put each per-cpu kdump ELF notes into a single page
Date: Thu, 11 Sep 2014 16:01:10 -0400	[thread overview]
Message-ID: <20140911200110.GA24699@redhat.com> (raw)
In-Reply-To: <20140905183314.23690175@hananiah.suse.cz>

On Fri, Sep 05, 2014 at 06:33:14PM +0200, Petr Tesarik wrote:
> On architectures that use percpu-vm, the percpu region is not guaranteed
> to be contiguous in physical space.

Petr,

Which are those arches?

> However, fs/proc/vmcore.c expects
> all ELF notes to be contiguous. If the ELF note happens to occupy
> two non-adjacent physical pages, part of the note may be read from an
> incorrect memory location by the kdump kernel, resulting in failure to
> initialize /proc/vmcore (if the content of the following physical page,
> incorrectly interpreted as an ELF note specifies a large number), wrong
> register values or other apparent random memory corruption.
> 
> There is currently no mechanism to pass the virtual-to-physical mapping
> of the percpu allocation to the kdump kernel. So, instead, I'm changing
> the alignment of the ELF note buffer. Since sizeof(note_buf_t) is less
> than PAGE_SIZE, aligning the buffer to the nearest higher power of 2
> is enough to make sure that the buffer cannot cross a page boundary,
> effectively ensuring that the whole buffer is contiguous in physical
> space.
> 
> Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
> ---
>  kernel/kexec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 2bee072..cdab59d 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -1610,7 +1610,8 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
>  static int __init crash_notes_memory_init(void)
>  {
>  	/* Allocate memory for saving cpu registers. */
> -	crash_notes = alloc_percpu(note_buf_t);
> +	crash_notes = __alloc_percpu(sizeof(note_buf_t),
> +				     roundup_pow_of_two(sizeof(note_buf_t)));

I think some of the changelog should show up here as comment in short
form. I don't think it is obvious that why we are using __alloc_percpu()
and why aligning to nearst higher power of 2 is needed here. Please also
mention here which arches run into issues.

Thanks
Vivek

>  	if (!crash_notes) {
>  		pr_warn("Kexec: Memory allocation for saving cpu register states failed\n");
>  		return -ENOMEM;
> -- 
> 1.8.4.5
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

  parent reply	other threads:[~2014-09-11 20:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-05 16:33 [PATCH] Put each per-cpu kdump ELF notes into a single page Petr Tesarik
2014-09-05 16:33 ` Petr Tesarik
2014-09-11 19:37 ` Petr Tesarik
2014-09-11 19:37   ` Petr Tesarik
2014-09-11 20:01 ` Vivek Goyal [this message]
2014-09-11 20:01   ` Vivek Goyal
2014-09-11 20:43   ` Petr Tesarik
2014-09-11 20:43     ` Petr Tesarik
2014-09-11 21:16     ` Vivek Goyal
2014-09-11 21:16       ` Vivek Goyal
2014-09-11 22:15       ` Petr Tesarik
2014-09-11 22:15         ` Petr Tesarik
2014-09-12 11:52         ` Vivek Goyal
2014-09-12 11:52           ` Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140911200110.GA24699@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ptesarik@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.