On Sep 18, 2016 8:11 AM, "Denys Vlasenko" <dvlas...@redhat.com> wrote: > > On 09/18/2016 10:31 AM, Ingo Molnar wrote: >> >> * Denys Vlasenko <dvlas...@redhat.com> wrote: >>> >>> On 09/15/2016 09:04 AM, Ingo Molnar wrote: >>>> >>>> * Denys Vlasenko <dvlas...@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? >>> >>> >>> 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.