lkml.org 
[lkml]   [2018]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 4/4] mm/sparse: Optimize memmap allocation during sparse_init()
From
Date
> @@ -297,8 +298,8 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
> if (!present_section_nr(pnum))
> continue;
>
> - map_map[pnum] = sparse_mem_map_populate(pnum, nodeid, NULL);
> - if (map_map[pnum])
> + map_map[nr_consumed_maps] = sparse_mem_map_populate(pnum, nodeid, NULL);
> + if (map_map[nr_consumed_maps++])
> continue;
...

This looks wonky.

This seems to say that even if we fail to sparse_mem_map_populate() (it
returns NULL), we still consume a map. Is that right?

> /* fallback */
> + nr_consumed_maps = 0;
> for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> struct mem_section *ms;
>
> if (!present_section_nr(pnum))
> continue;
> - map_map[pnum] = sparse_mem_map_populate(pnum, nodeid, NULL);
> - if (map_map[pnum])
> + map_map[nr_consumed_maps] = sparse_mem_map_populate(pnum, nodeid, NULL);
> + if (map_map[nr_consumed_maps++])
> continue;

Same questionable pattern as above...

> #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> - size2 = sizeof(struct page *) * NR_MEM_SECTIONS;
> + size2 = sizeof(struct page *) * nr_present_sections;
> map_map = memblock_virt_alloc(size2, 0);
> if (!map_map)
> panic("can not allocate map_map\n");
> @@ -586,27 +594,44 @@ void __init sparse_init(void)
> sizeof(map_map[0]));
> #endif
>
> + /* The numner of present sections stored in nr_present_sections

"number"?

Also, this is not correct comment CodingStyle.

> + * are kept the same since mem sections are marked as present in
> + * memory_present().

Are you just trying to say that we are not making sections present here?

> In this for loop, we need check which sections
> + * failed to allocate memmap or usemap, then clear its
> + * ->section_mem_map accordingly. During this process, we need
> + * increase 'alloc_usemap_and_memmap' whether its allocation of
> + * memmap or usemap failed or not, so that after we handle the i-th
> + * memory section, can get memmap and usemap of (i+1)-th section
> + * correctly. */

I'm really scratching my head over this comment. For instance "increase
'alloc_usemap_and_memmap'" doesn't make any sense to me. How do you
increase a function?

I wonder if you could give that comment another shot.

\
 
 \ /
  Last update: 2018-06-08 00:48    [W:0.097 / U:2.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site