Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> writes: > Hi Ying, > > On 2023/10/23 09:18, Huang, Ying wrote: >> Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> writes: >> >>> Hi Ying, >>> >>> On 2023/10/20 15:05, Huang, Ying wrote: >>>> Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> writes: >>>> >>>>> In offline_pages(), if a node becomes memoryless, we >>>>> will clear its N_MEMORY state by calling node_states_clear_node(). >>>>> But we do this after rebuilding the zonelists by calling >>>>> build_all_zonelists(), which will cause this memoryless node to >>>>> still be in the fallback list of other nodes. >>>> For fallback list, do you mean pgdat->node_zonelists[]? If so, in >>>> build_all_zonelists >>>> __build_all_zonelists >>>> build_zonelists >>>> build_zonelists_in_node_order >>>> build_zonerefs_node >>>> populated_zone() will be checked before adding zone into zonelist. >>>> So, IIUC, we will not try to allocate from the memory less node. >>> >>> Normally yes, but if it is the weird topology mentioned in [1], it's >>> possible to allocate memory from it, it is a memoryless node, but it >>> also has memory. >>> >>> In addition to the above case, I think it's reasonable to remove >>> memory less node from node_order[] in advance. In this way it will >>> not to be traversed in build_zonelists_in_node_order(). >>> >>> [1]. https://lore.kernel.org/all/20230212110305.93670-1-zhengqi.arch@xxxxxxxxxxxxx/ >> Got it! Thank you for information. I think that it may be good to >> include this in the patch description to avoid potential confusing in >> the future. > > OK, maybe the commit message can be changed to the following: > > ``` > In offline_pages(), if a node becomes memoryless, we > will clear its N_MEMORY state by calling node_states_clear_node(). > But we do this after rebuilding the zonelists by calling > build_all_zonelists(), which will cause this memoryless node to > still be in the fallback nodes (node_order[]) of other nodes. > > To drop memoryless nodes from fallback nodes in this case, just > call node_states_clear_node() before calling build_all_zonelists(). > > In this way, we will not try to allocate pages from memoryless > node0, then the panic mentioned in [1] will also be fixed. Even though > this problem has been solved by dropping the NODE_MIN_SIZE constrain > in x86 [2], it would be better to fix it in the core MM as well. > > [1]. https://lore.kernel.org/all/20230212110305.93670-1-zhengqi.arch@xxxxxxxxxxxxx/ > [2]. https://lore.kernel.org/all/20231017062215.171670-1-rppt@xxxxxxxxxx/ > > ``` This is helpful. Thanks! -- Best Regards, Huang, Ying > Thanks, > Qi > >> -- >> Best Regards, >> Huang, Ying >> >>> Thanks, >>> Qi >>> >>> >>>> -- >>>> Best Regards, >>>> Huang, Ying >>>> >>>>> This will incur >>>>> some runtime overhead. >>>>> >>>>> To drop memoryless node from fallback lists in this case, just >>>>> call node_states_clear_node() before calling build_all_zonelists(). >>>>> >>>>> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> >>>>> Acked-by: David Hildenbrand <david@xxxxxxxxxx> >>>> [snip] >>>> -- >>>> Best Regards, >>>> Huang, Ying
