> @@ -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.
