On 6/26/20 6:02 AM, Joonsoo Kim wrote: > 2020년 6월 25일 (목) 오후 8:26, Michal Hocko <mhocko@xxxxxxxxxx>님이 작성: >> >> On Tue 23-06-20 15:13:43, Joonsoo Kim wrote: >> > From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> >> > >> > There is no difference between two migration callback functions, >> > alloc_huge_page_node() and alloc_huge_page_nodemask(), except >> > __GFP_THISNODE handling. This patch adds an argument, gfp_mask, on >> > alloc_huge_page_nodemask() and replace the callsite for >> > alloc_huge_page_node() with the call to >> > alloc_huge_page_nodemask(..., __GFP_THISNODE). >> > >> > It's safe to remove a node id check in alloc_huge_page_node() since >> > there is no caller passing NUMA_NO_NODE as a node id. >> >> Yes this is indeed safe. alloc_huge_page_node used to be called from >> other internal hugetlb allocation layer and that allowed NUMA_NO_NODE as >> well. Now it is called only from the mempolicy migration callback and >> that always specifies a node and want to stick with that node. >> >> But I have to say I really dislike the gfp semantic because it is >> different from any other allocation function I can think of. It >> specifies what to be added rather than what should be used. >> >> Removing the function is ok but please use the full gfp mask instead >> or if that is impractical for some reason (wich shouldn't be the case >> as htlb_alloc_mask should be trivial to make static inline) make it >> explicit that this is not a gfp_mask but a gfp modifier and explicitly >> state which modifiers are allowed. > > Okay. I will try to solve your concern. Concrete solution is not yet prepared > but perhaps I will use full gfp_mask by using htlb_alloc_mask() in caller sites. Yeah, that should be feasible. alloc_huge_page_vma() already does htlb_alloc_mask(h). In alloc_new_node_page() and new_page_nodemask() it would be consistent with the other cases handled there (THP and base). > Thanks. >
