On 07/01/18 at 10:18pm, Pavel Tatashin wrote: > > Here, I think it might be not right to jump to 'failed' directly if one > > section of the node failed to populate memmap. I think the original code > > is only skipping the section which memmap failed to populate by marking > > it as not present with "ms->section_mem_map = 0". > > > > Hi Baoquan, > > Thank you for a careful review. This is an intended change compared to > the original code. Because we operate per-node now, if we fail to > allocate a single section, in this node, it means we also will fail to > allocate all the consequent sections in the same node and no need to > check them anymore. In the original code we could not simply bailout, > because we still might have valid entries in the following nodes. > Similarly, sparse_init() will call sparse_init_nid() for the next node > even if previous node failed to setup all the memory.
Hmm, say the node we are handling is node5, and there are 100 sections. If you allocate memmap for section at one time, you have succeeded to handle for the first 99 sections, now the 100th failed, so you will mark all sections on node5 as not present. And the allocation failure is only for single section memmap allocation case. Please think about the vmemmap case, it will map the struct page pages to vmemmap, and will populate page tables for them to map. That is a long walk, not only memmory allocation, and page table checking and populating, one section failing to populate memmap doesn't mean all the consequent sections also failed. I think the original code is reasonable. Thanks Baoquan
