Re: [PATCH 1/2] mm, thp: introduce dedicated transparent huge page allocation interfaces

["Du, Changbin" <changbin.du@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]

Hi Hocko,
On Thu, Oct 19, 2017 at 02:49:31PM +0200, Michal Hocko wrote:
> On Wed 18-10-17 19:00:26, Du, Changbin wrote:
> > Hi Hocko,
> > 
> > On Tue, Oct 17, 2017 at 12:20:52PM +0200, Michal Hocko wrote:
> > > [CC Kirill]
> > > 
> > > On Mon 16-10-17 17:19:16, changbin.du@intel.com wrote:
> > > > From: Changbin Du <changbin.du@intel.com>
> > > > 
> > > > This patch introduced 4 new interfaces to allocate a prepared
> > > > transparent huge page.
> > > >   - alloc_transhuge_page_vma
> > > >   - alloc_transhuge_page_nodemask
> > > >   - alloc_transhuge_page_node
> > > >   - alloc_transhuge_page
> > > > 
> > > > The aim is to remove duplicated code and simplify transparent
> > > > huge page allocation. These are similar to alloc_hugepage_xxx
> > > > which are for hugetlbfs pages. 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
> > > 
> > > So what exactly is the advantage of the new API? The diffstat doesn't
> > > sound very convincing to me.
> > >
> > The caller only need one step to allocate thp. Several LOCs removed for all the
> > caller side with this change. So it's little more convinent.
> 
> Yeah, but the overall result is more code. So I am not really convinced. 
Yes, but some of code are just to make compiler happy (declarations). These are
just simple light wrappers same as other functions in kernel. At least the code
readbility is improved by this, two steps allocation merged into one so
duplicated logic removed.

> -- 
> Michal Hocko
> SUSE Labs

-- 
Thanks,
Changbin Du

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]