On Mon, Jun 6, 2022 at 1:59 PM Yang Shi <shy828301@xxxxxxxxx> wrote:
>
> On Fri, Jun 3, 2022 at 5:40 PM Zach O'Keefe <zokeefe@xxxxxxxxxx> wrote:
> >
> > Add a gfp_t flags member to struct collapse_control that allows contexts
> > to specify their own allocation semantics. This decouples the
> > allocation semantics from
> > /sys/kernel/mm/transparent_hugepage/khugepaged/defrag.
> >
> > khugepaged updates this member for every hugepage processed, since the
> > sysfs setting might change at any time.
> >
> > Signed-off-by: Zach O'Keefe <zokeefe@xxxxxxxxxx>
> > ---
> > mm/khugepaged.c | 21 +++++++++++++--------
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 38488d114073..ba722347bebd 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -92,6 +92,9 @@ struct collapse_control {
> >
> > /* Last target selected in khugepaged_find_target_node() */
> > int last_target_node;
> > +
> > + /* gfp used for allocation and memcg charging */
> > + gfp_t gfp;
> > };
> >
> > /**
> > @@ -994,15 +997,14 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm,
> > return true;
> > }
> >
> > -static int alloc_charge_hpage(struct page **hpage, struct mm_struct *mm,
> > +static int alloc_charge_hpage(struct mm_struct *mm, struct page **hpage,
>
> Why did you have to reverse the order of mm and hpage? It seems
> pointless and you could save a couple of changed lines.
At some point many versions ago I must have thought it was cleaner to
be consistent with struct *mm as 1st arg, but agree with unnecessary
change and have removed.
> > struct collapse_control *cc)
> > {
> > - gfp_t gfp = alloc_hugepage_khugepaged_gfpmask() | __GFP_THISNODE;
> > int node = khugepaged_find_target_node(cc);
> >
> > - if (!khugepaged_alloc_page(hpage, gfp, node))
> > + if (!khugepaged_alloc_page(hpage, cc->gfp, node))
> > return SCAN_ALLOC_HUGE_PAGE_FAIL;
> > - if (unlikely(mem_cgroup_charge(page_folio(*hpage), mm, gfp)))
> > + if (unlikely(mem_cgroup_charge(page_folio(*hpage), mm, cc->gfp)))
> > return SCAN_CGROUP_CHARGE_FAIL;
> > count_memcg_page_event(*hpage, THP_COLLAPSE_ALLOC);
> > return SCAN_SUCCEED;
> > @@ -1032,7 +1034,7 @@ static void collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > */
> > mmap_read_unlock(mm);
> >
> > - result = alloc_charge_hpage(hpage, mm, cc);
> > + result = alloc_charge_hpage(mm, hpage, cc);
> > if (result != SCAN_SUCCEED)
> > goto out_nolock;
> >
> > @@ -1613,7 +1615,7 @@ static void collapse_file(struct mm_struct *mm, struct file *file,
> > VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> > VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
> >
> > - result = alloc_charge_hpage(hpage, mm, cc);
> > + result = alloc_charge_hpage(mm, hpage, cc);
> > if (result != SCAN_SUCCEED)
> > goto out;
> >
> > @@ -2037,8 +2039,7 @@ static void khugepaged_scan_file(struct mm_struct *mm, struct file *file,
> > }
> > #else
> > static void khugepaged_scan_file(struct mm_struct *mm, struct file *file,
> > - pgoff_t start, struct page **hpage,
> > - struct collapse_control *cc)
> > + pgoff_t start, struct collapse_control *cc)
>
> Why was the !CONFIG_SHMEM version definition changed, but CONFIG_SHMEM
> version was not?
Definitely an error - sorry about that. Surprised I didn't get caught
by this earlier - must be because the signatures are eventually made
equal when struct **hpage is dropped. Have fixed it.
> > {
> > BUILD_BUG();
> > }
> > @@ -2121,6 +2122,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
> > if (unlikely(khugepaged_test_exit(mm)))
> > goto breakouterloop;
> >
> > + /* reset gfp flags since sysfs settings might change */
> > + cc->gfp = alloc_hugepage_khugepaged_gfpmask() |
> > + __GFP_THISNODE;
> > VM_BUG_ON(khugepaged_scan.address < hstart ||
> > khugepaged_scan.address + HPAGE_PMD_SIZE >
> > hend);
> > @@ -2255,6 +2259,7 @@ static int khugepaged(void *none)
> > struct mm_slot *mm_slot;
> > struct collapse_control cc = {
> > .last_target_node = NUMA_NO_NODE,
> > + /* .gfp set later */
>
> Seems pointless to me.
Ack. Removed.
> > };
> >
> > set_freezable();
> > --
> > 2.36.1.255.ge46751e96f-goog
> >
