Re: [PATCH v4] mm, thp: introduce generic transparent huge page allocation interfaces

From: Du, Changbin
Date: Fri Dec 08 2017 - 22:35:08 EST


On Fri, Dec 08, 2017 at 09:27:37AM +0100, Michal Hocko wrote:
> On Fri 08-12-17 12:42:55, changbin.du@xxxxxxxxx wrote:
> > From: Changbin Du <changbin.du@xxxxxxxxx>
> >
> > This patch introduced 4 new interfaces to allocate a prepared transparent
> > huge page. These interfaces merge distributed two-step allocation as simple
> > single step. And they can avoid issue like forget to call prep_transhuge_page()
> > or call it on wrong page. A real fix:
> > 40a899e ("mm: migrate: fix an incorrect call of prep_transhuge_page()")
> >
> > Anyway, I just want to prove that expose direct allocation interfaces is
> > better than a interface only do the second part of it.
> >
> > These are similar to alloc_hugepage_xxx which are for hugetlbfs pages. New
> > interfaces are:
> > - alloc_transhuge_page_vma
> > - alloc_transhuge_page_nodemask
> > - alloc_transhuge_page_node
> > - alloc_transhuge_page
> >
> > These interfaces implicitly add __GFP_COMP gfp mask which is the minimum
> > flags used for huge page allocation. More flags leave to the callers.
> >
> > This patch does below changes:
> > - define alloc_transhuge_page_xxx interfaces
> > - apply them to all existing code
> > - declare prep_transhuge_page as static since no others use it
> > - remove alloc_hugepage_vma definition since it no longer has users
>
> I am not really convinced this is a huge win, to be honest. Just look at
> the diffstat. Very few callsites get marginally simpler while we add a
> lot of stubs and the code churn.
>
I know we should write less code, but it is not the only rule. Sometimes we need
add little more code since the compiler requires so, but it doesn't mean then
the compiler will generate worse/more machine code. Besides this, I really want
to know wethere any other considerations you have. Thanks.

> > mm/mempolicy.c | 14 +++-----------
> > mm/migrate.c | 14 ++++----------
> > mm/shmem.c | 6 ++----
> > 8 files changed, 90 insertions(+), 56 deletions(-)
> --
> Michal Hocko
> SUSE Labs

--
Thanks,
Changbin Du