Re: [PATCH 2/2 v2] x86/e820: Use much less memory for e820/e820_saved, save up to 120k

[Andy Lutomirski]

On Sep 18, 2016 8:11 AM, "Denys Vlasenko" <dvlasenk@xxxxxxxxxx> wrote:
>
> On 09/18/2016 10:31 AM, Ingo Molnar wrote:
>>
>> * Denys Vlasenko <dvlasenk@xxxxxxxxxx> wrote:
>>>
>>> On 09/15/2016 09:04 AM, Ingo Molnar wrote:
>>>>
>>>> * Denys Vlasenko <dvlasenk@xxxxxxxxxx> 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?
>
>
> __ro_after_init makes sense only for statically allocated objects.
> e820_reallocate_tables() copies e280 maps to kmalloced memory.

The pointer can be __ro_after_init, though.  You could also
set_memory_ro() the thing if it's big enough that page-aligning it
wouldn't be a problem.