lkml.org 
[lkml]   [2020]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 3/8] mm/hugetlb: unify migration callbacks
    From
    Date
    On 6/26/20 6:02 AM, Joonsoo Kim wrote:
    > 2020년 6월 25일 (목) 오후 8:26, Michal Hocko <mhocko@kernel.org>님이 작성:
    >>
    >> On Tue 23-06-20 15:13:43, Joonsoo Kim wrote:
    >> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    >> >
    >> > 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.
    >

    \
     
     \ /
      Last update: 2020-07-02 18:14    [W:3.304 / U:1.420 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site