linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	"H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@alien8.de>,
	Brian Gerst <brgerst@gmail.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2 v2] x86/e820: Use much less memory for e820/e820_saved, save up to 120k
Date: Thu, 15 Sep 2016 09:04:30 +0200	[thread overview]
Message-ID: <20160915070430.GA25643@gmail.com> (raw)
In-Reply-To: <20160909075827.14602-1-dvlasenk@redhat.com>


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> The maximum size of e820 map array for EFI systems is defined as
> E820_X_MAX (E820MAX + 3 * MAX_NUMNODES).
> 
> In x86_64 defconfig, this ends up with E820_X_MAX = 320, e820 and e820_saved
> are 6404 bytes each.
> 
> With larger configs, for example Fedora kernels, E820_X_MAX = 3200, e820 and e820_saved
> are 64004 bytes each. Most of this space is wasted. Typical machines have some 20-30
> e820 areas at most.
> 
> This patch turns e820 and e820_saved to pointers which initially point to __initdata
> tables, of the same size as before.
> 
> At the very end of setup_arch(), when we are done fiddling with these maps,
> allocate smaller alloc_bootmem blocks, copy maps there, and change pointers.
> 
> Run-tested.

> +/*
> + * Initial e820 and e820_saved are largish __initdata arrays.
> + * Copy them to (usually much smaller) dynamically allocated area.
> + * This is done after all tweaks we ever do to them.
> + */
> +__init void e820_reallocate_tables(void)
> +{
> +	struct e820map *n;
> +	int size;
> +
> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820->nr_map;
> +	n = alloc_bootmem(size);
> +	memcpy(n, e820, size);
> +	e820 = n;
> +
> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820_saved->nr_map;
> +	n = alloc_bootmem(size);
> +	memcpy(n, e820_saved, size);
> +	e820_saved = n;
> +}

Ok, this makes me quite nervous, could you please split this into two patches so 
that any fails can be nicely bisected to?

First patch only does the pointerization changes with a trivial placeholder 
structure (full size, static allocated), second patch does all the dangerous bits 
such as changing it to __initdata, allocating and copying over bits.

Also, could we please also add some minimal debugging facility to make sure the 
memory table does not get extended after it's been reallocated?

Thanks,

	Ingo

  reply	other threads:[~2016-09-15  7:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-09  7:58 [PATCH 2/2 v2] x86/e820: Use much less memory for e820/e820_saved, save up to 120k Denys Vlasenko
2016-09-15  7:04 ` Ingo Molnar [this message]
2016-09-17 21:09   ` Denys Vlasenko
2016-09-18  8:31     ` Ingo Molnar
2016-09-18 18:11       ` Denys Vlasenko
2016-09-18 19:58         ` Andy Lutomirski

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=20160915070430.GA25643@gmail.com \
    --to=mingo@kernel.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).