Re: [PATCH v6 04/15] mm/khugepaged: dedup and simplify hugepage alloc and charging

[Zach O'Keefe <zokeefe@google.com>]

On Jun 29 17:58, Peter Xu wrote:
> On Fri, Jun 03, 2022 at 05:39:53PM -0700, Zach O'Keefe wrote:
> > The following code is duplicated in collapse_huge_page() and
> > collapse_file():
> > 
> >         gfp = alloc_hugepage_khugepaged_gfpmask() | __GFP_THISNODE;
> > 
> > 	new_page = khugepaged_alloc_page(hpage, gfp, node);
> >         if (!new_page) {
> >                 result = SCAN_ALLOC_HUGE_PAGE_FAIL;
> >                 goto out;
> >         }
> > 
> >         if (unlikely(mem_cgroup_charge(page_folio(new_page), mm, gfp))) {
> >                 result = SCAN_CGROUP_CHARGE_FAIL;
> >                 goto out;
> >         }
> >         count_memcg_page_event(new_page, THP_COLLAPSE_ALLOC);
> > 
> > Also, "node" is passed as an argument to both collapse_huge_page() and
> > collapse_file() and obtained the same way, via
> > khugepaged_find_target_node().
> > 
> > Move all this into a new helper, alloc_charge_hpage(), and remove the
> > duplicate code from collapse_huge_page() and collapse_file().  Also,
> > simplify khugepaged_alloc_page() by returning a bool indicating
> > allocation success instead of a copy of the allocated struct page.
> > 
> > Suggested-by: Peter Xu <peterx@redhat.com>
> > 
> > ---
> 
> [note: please remember to drop this "---" when repost since I think it
>  could drop your sign-off when apply]
>

Thanks for catching this, Peter! Fixed locally!

Best,
Zach

> > 
> > Signed-off-by: Zach O'Keefe <zokeefe@google.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Thanks,
> 
> -- 
> Peter Xu
>