lkml.org 
[lkml]   [2020]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 00/12] clean-up the migration target allocation functions
    2020년 5월 29일 (금) 오전 4:25, Vlastimil Babka <vbabka@suse.cz>님이 작성:
    >
    > On 5/27/20 8:44 AM, js1304@gmail.com wrote:
    > > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    > >
    > > This patchset clean-up the migration target allocation functions.
    > >
    > > * Changes on v2
    > > - add acked-by tags
    > > - fix missing compound_head() call for the patch #3
    > > - remove thisnode field on alloc_control and use __GFP_THISNODE directly
    > > - fix missing __gfp_mask setup for the patch
    > > "mm/hugetlb: do not modify user provided gfp_mask"
    > >
    > > * Cover-letter
    > >
    > > Contributions of this patchset are:
    > > 1. unify two hugetlb alloc functions. As a result, one is remained.
    > > 2. make one external hugetlb alloc function to internal one.
    > > 3. unify three functions for migration target allocation.
    > >
    > > The patchset is based on next-20200526.
    > > The patchset is available on:
    >
    > I went through the series and I'd like to make some high-level suggestions
    > first, that should hopefully simplify the code a bit more and reduce churn:

    Thanks for review!
    I have not enough time today to check your suggestions.
    I will check on next week and then reply again.

    Thanks.

    > - in the series, alloc_huge_page_nodemask() becomes the only caller of
    > alloc_migrate_huge_page(). So you can inline the code there, and it's one less
    > function out of many with similar name :)
    >
    > - after that, alloc_huge_page_nodemask(ac) uses ac mostly just to extract
    > individual fields, and only pass it as a whole to dequeue_huge_page_nodemask().
    > The only other caller of dequeue...() is dequeue_huge_page_vma() who has to
    > construct ac from scratch. It might be probably simpler not to introduce struct
    > alloc_control into hugetlb code at all, and only keep it for
    > alloc_migrate_target(), at which point it can have a more specific name as
    > discussed and there's less churn
    >
    > - I'd suggest not change signature of migrate_pages(), free_page_t and
    > new_page_t, keeping the opaque private field is fine as not all callbacks use
    > struct alloc_context pointer, and then e.g. compaction_alloc has to use the
    > private field etc. alloc_migration_target() can simply cast the private to
    > struct alloc_control *ac as the first thing

    \
     
     \ /
      Last update: 2020-05-29 08:51    [W:2.984 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site