From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755538AbcIRIbm (ORCPT ); Sun, 18 Sep 2016 04:31:42 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36670 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752803AbcIRIbd (ORCPT ); Sun, 18 Sep 2016 04:31:33 -0400 Date: Sun, 18 Sep 2016 10:31:27 +0200 From: Ingo Molnar To: Denys Vlasenko Cc: Andy Lutomirski , "H. Peter Anvin" , Borislav Petkov , Brian Gerst , 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 Message-ID: <20160918083127.GA15186@gmail.com> References: <20160909075827.14602-1-dvlasenk@redhat.com> <20160915070430.GA25643@gmail.com> <494d4f1a-272f-4ee4-d184-6d6645584012@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <494d4f1a-272f-4ee4-d184-6d6645584012@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Denys Vlasenko wrote: > On 09/15/2016 09:04 AM, Ingo Molnar wrote: > > > >* Denys Vlasenko 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? > > No problem. > > >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? > > I have another idea: run e820_reallocate_tables() later, just before > we free __init and __initdata. Then e820 tables _can't_ be_ changed - > all functions which do that are __init functions. > > Will test this now, and send a patchset. Could we also mark it __ro_after_init? Thanks, Ingo