lkml.org 
[lkml]   [2018]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v3 4/4] mm/sparse: Optimize memmap allocation during sparse_init()
From
Date
I'm having a really hard time tying all the pieces back together.  Let
me give it a shot and you can tell me where I go wrong.

On 02/27/2018 07:26 PM, Baoquan He wrote:
> In sparse_init(), two temporary pointer arrays, usemap_map and map_map
> are allocated with the size of NR_MEM_SECTIONS.

In sparse_init(), two temporary pointer arrays, usemap_map and map_map
are allocated to hold the maps for every possible memory section
(NR_MEM_SECTIONS). However, we obviously only need the array sized for
nr_present_sections (introduced in patch 1).

The reason this is a problem is that, with 5-level paging,
NR_MEM_SECTIONS (8M->512M) went up dramatically and these temporary
arrays can eat all of memory, like on kdump kernels.

This patch does two things: it makes sure to give usemap_map/mem_map a
less gluttonous size on small systems, and it changes the map allocation
and handling to handle the now more compact, less sparse arrays.

---

The code looks fine to me. It's a bit of a shame that there's no
verification to ensure that idx_present never goes beyond the shiny new
nr_present_sections.


> @@ -583,6 +592,7 @@ void __init sparse_init(void)
> unsigned long *usemap;
> unsigned long **usemap_map;
> int size;
> + int idx_present = 0;

I wonder whether idx_present is a good name. Isn't it the number of
consumed mem_map[]s or usemaps?

>
> if (!map) {
> ms->section_mem_map = 0;
> + idx_present++;
> continue;
> }
>


This hunk seems logically odd to me. I would expect a non-used section
to *not* consume an entry from the temporary array. Why does it? The
error and success paths seem to do the same thing.

\
 
 \ /
  Last update: 2018-04-06 16:51    [W:0.186 / U:2.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site