Messages in this thread | | | Date | Mon, 23 Oct 2023 10:53:08 +0800 | Subject | Re: [PATCH v3 2/2] mm: memory_hotplug: drop memoryless node from fallback lists | From | Qi Zheng <> |
| |
Hi Ying,
On 2023/10/23 09:18, Huang, Ying wrote: > Qi Zheng <zhengqi.arch@bytedance.com> writes: > >> Hi Ying, >> >> On 2023/10/20 15:05, Huang, Ying wrote: >>> Qi Zheng <zhengqi.arch@bytedance.com> 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@bytedance.com/ > > 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@bytedance.com/ [2]. https://lore.kernel.org/all/20231017062215.171670-1-rppt@kernel.org/
```
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@bytedance.com> >>>> Acked-by: David Hildenbrand <david@redhat.com> >>> [snip] >>> -- >>> Best Regards, >>> Huang, Ying
| |