From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4CCD8C43334 for ; Mon, 6 Jun 2022 20:51:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDB626B0083; Mon, 6 Jun 2022 16:51:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CB1F66B0085; Mon, 6 Jun 2022 16:51:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BA11D6B0087; Mon, 6 Jun 2022 16:51:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id AC1AD6B0083 for ; Mon, 6 Jun 2022 16:51:12 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id 8247C80A11 for ; Mon, 6 Jun 2022 20:51:12 +0000 (UTC) X-FDA: 79549005984.18.B9B52B6 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf04.hostedemail.com (Postfix) with ESMTP id AAE4240039 for ; Mon, 6 Jun 2022 20:50:50 +0000 (UTC) Received: by mail-pl1-f173.google.com with SMTP id b5so13057945plx.10 for ; Mon, 06 Jun 2022 13:51:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RymWhcPlakpR2ZRf+jLSsPrFlL7bY1wqYW9BMOmBpuQ=; b=I8uHPARl1Amslu4qZ4j1BloVavHzwHmX/XrX0Ub90P6dGdruuL987RwqW1U33wavMw rovq/PWtBuydr3UudJ8aClhfNhA+IkoFEqmpGLfkzae53SVq0S4VdiyzL+/4WRgjBr2g VXhuUGupkwWh2pxYZpicDJlZrh3eqQnBhjiF2o7EH3Hvm+V81LUvjScRpdl4wW5IAEl7 f+Pa4i+GXQOBm38WugBRI5Pc7iYqHYbc/akUBawrnlCDF3bg3UqrdWzkoxgePvrQqIjH c9niL40aMQkAZp5QakD8gaQv8n5/T+QVer4TAB2LZ1t54U2caWWVC1hAAa3aHASjGqq/ anZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RymWhcPlakpR2ZRf+jLSsPrFlL7bY1wqYW9BMOmBpuQ=; b=r4exvphZNLoNEWdpBXY8qxzn3gj4pCMypDxHvsoD990oJrdr2zB58fsMFooB7YAy+Z nMRvcdz0dBcf55s/8dT4TTTFFQRdxQHjdAeX6Fsm+hpGxt8Sy3xxzeIMnoiSV1GeJnLZ /CCltakizq71X6FUXmKuFV5eBivLv1jg4BXXW1afA4sik2LEfLXRe32xrfhCktTMQrIQ 1lLNjbHLUvCk9j8+iYOQCRjnDtQUTZ35IUxD/no9/T/uWJ9hEbVu0KfJMmaRVRRu7pLg oSgSiwfZH3RTBNMsvzc3j3NtXLhXln9NwDMWefFSBykmbOJiTUGZL0PyXV/tiblz9+bW ZFYg== X-Gm-Message-State: AOAM5338T/Q3lGAStGoELzQHMJvn5U93aNp0DvFpXIbv8zEcdEx3k51p jZPieG3j3tg5yG/vVqTus3B+dddKo29oVMTLtVk= X-Google-Smtp-Source: ABdhPJwwBpyknr13BQW1vx+29Z9Y3vP8IzYUve+sAy5HN9/DQgEfCcSfaPb69vmGWcFrwWxDV3qJ0Lv+taHSaLEjrag= X-Received: by 2002:a17:903:32c4:b0:167:6e6f:204b with SMTP id i4-20020a17090332c400b001676e6f204bmr11351742plr.117.1654548670808; Mon, 06 Jun 2022 13:51:10 -0700 (PDT) MIME-Version: 1.0 References: <20220604004004.954674-1-zokeefe@google.com> <20220604004004.954674-5-zokeefe@google.com> In-Reply-To: <20220604004004.954674-5-zokeefe@google.com> From: Yang Shi Date: Mon, 6 Jun 2022 13:50:58 -0700 Message-ID: Subject: Re: [PATCH v6 04/15] mm/khugepaged: dedup and simplify hugepage alloc and charging To: "Zach O'Keefe" Cc: Alex Shi , David Hildenbrand , David Rientjes , Matthew Wilcox , Michal Hocko , Pasha Tatashin , Peter Xu , Rongwei Wang , SeongJae Park , Song Liu , Vlastimil Babka , Zi Yan , Linux MM , Andrea Arcangeli , Andrew Morton , Arnd Bergmann , Axel Rasmussen , Chris Kennelly , Chris Zankel , Helge Deller , Hugh Dickins , Ivan Kokshaysky , "James E.J. Bottomley" , Jens Axboe , "Kirill A. Shutemov" , Matt Turner , Max Filippov , Miaohe Lin , Minchan Kim , Patrick Xia , Pavel Begunkov , Thomas Bogendoerfer Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=I8uHPARl; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf04.hostedemail.com: domain of shy828301@gmail.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Stat-Signature: peo9u3565ya3t99ibh96isx7nc1pqwhz X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: AAE4240039 X-HE-Tag: 1654548650-639002 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Jun 3, 2022 at 5:40 PM 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 > > --- > > Signed-off-by: Zach O'Keefe Reviewed-by: Yang Shi > --- > mm/khugepaged.c | 77 ++++++++++++++++++++++--------------------------- > 1 file changed, 34 insertions(+), 43 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 907d0b2bd4bd..38488d114073 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -860,19 +860,18 @@ static bool alloc_fail_should_sleep(struct page **hpage, bool *wait) > return false; > } > > -static struct page * > -khugepaged_alloc_page(struct page **hpage, gfp_t gfp, int node) > +static bool khugepaged_alloc_page(struct page **hpage, gfp_t gfp, int node) > { > *hpage = __alloc_pages_node(node, gfp, HPAGE_PMD_ORDER); > if (unlikely(!*hpage)) { > count_vm_event(THP_COLLAPSE_ALLOC_FAILED); > *hpage = ERR_PTR(-ENOMEM); > - return NULL; > + return false; > } > > prep_transhuge_page(*hpage); > count_vm_event(THP_COLLAPSE_ALLOC); > - return *hpage; > + return true; > } > > /* > @@ -995,10 +994,23 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm, > return true; > } > > -static void collapse_huge_page(struct mm_struct *mm, > - unsigned long address, > - struct page **hpage, > - int node, int referenced, int unmapped) > +static int alloc_charge_hpage(struct page **hpage, struct mm_struct *mm, > + 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)) > + return SCAN_ALLOC_HUGE_PAGE_FAIL; > + if (unlikely(mem_cgroup_charge(page_folio(*hpage), mm, gfp))) > + return SCAN_CGROUP_CHARGE_FAIL; > + count_memcg_page_event(*hpage, THP_COLLAPSE_ALLOC); > + return SCAN_SUCCEED; > +} > + > +static void collapse_huge_page(struct mm_struct *mm, unsigned long address, > + struct page **hpage, int referenced, > + int unmapped, struct collapse_control *cc) > { > LIST_HEAD(compound_pagelist); > pmd_t *pmd, _pmd; > @@ -1009,13 +1021,9 @@ static void collapse_huge_page(struct mm_struct *mm, > int isolated = 0, result = 0; > struct vm_area_struct *vma; > struct mmu_notifier_range range; > - gfp_t gfp; > > VM_BUG_ON(address & ~HPAGE_PMD_MASK); > > - /* Only allocate from the target node */ > - gfp = alloc_hugepage_khugepaged_gfpmask() | __GFP_THISNODE; > - > /* > * Before allocating the hugepage, release the mmap_lock read lock. > * The allocation can take potentially a long time if it involves > @@ -1023,17 +1031,12 @@ static void collapse_huge_page(struct mm_struct *mm, > * that. We will recheck the vma after taking it again in write mode. > */ > mmap_read_unlock(mm); > - new_page = khugepaged_alloc_page(hpage, gfp, node); > - if (!new_page) { > - result = SCAN_ALLOC_HUGE_PAGE_FAIL; > - goto out_nolock; > - } > > - if (unlikely(mem_cgroup_charge(page_folio(new_page), mm, gfp))) { > - result = SCAN_CGROUP_CHARGE_FAIL; > + result = alloc_charge_hpage(hpage, mm, cc); > + if (result != SCAN_SUCCEED) > goto out_nolock; > - } > - count_memcg_page_event(new_page, THP_COLLAPSE_ALLOC); > + > + new_page = *hpage; > > mmap_read_lock(mm); > result = hugepage_vma_revalidate(mm, address, &vma); > @@ -1306,10 +1309,9 @@ static int khugepaged_scan_pmd(struct mm_struct *mm, struct vm_area_struct *vma, > out_unmap: > pte_unmap_unlock(pte, ptl); > if (ret) { > - node = khugepaged_find_target_node(cc); > /* collapse_huge_page will return with the mmap_lock released */ > - collapse_huge_page(mm, address, hpage, node, > - referenced, unmapped); > + collapse_huge_page(mm, address, hpage, referenced, unmapped, > + cc); > } > out: > trace_mm_khugepaged_scan_pmd(mm, page, writable, referenced, > @@ -1578,7 +1580,7 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff) > * @file: file that collapse on > * @start: collapse start address > * @hpage: new allocated huge page for collapse > - * @node: appointed node the new huge page allocate from > + * @cc: collapse context and scratchpad > * > * Basic scheme is simple, details are more complex: > * - allocate and lock a new huge page; > @@ -1595,12 +1597,11 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff) > * + restore gaps in the page cache; > * + unlock and free huge page; > */ > -static void collapse_file(struct mm_struct *mm, > - struct file *file, pgoff_t start, > - struct page **hpage, int node) > +static void collapse_file(struct mm_struct *mm, struct file *file, > + pgoff_t start, struct page **hpage, > + struct collapse_control *cc) > { > struct address_space *mapping = file->f_mapping; > - gfp_t gfp; > struct page *new_page; > pgoff_t index, end = start + HPAGE_PMD_NR; > LIST_HEAD(pagelist); > @@ -1612,20 +1613,11 @@ static void collapse_file(struct mm_struct *mm, > VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem); > VM_BUG_ON(start & (HPAGE_PMD_NR - 1)); > > - /* Only allocate from the target node */ > - gfp = alloc_hugepage_khugepaged_gfpmask() | __GFP_THISNODE; > - > - new_page = khugepaged_alloc_page(hpage, gfp, node); > - if (!new_page) { > - result = SCAN_ALLOC_HUGE_PAGE_FAIL; > + result = alloc_charge_hpage(hpage, mm, cc); > + if (result != SCAN_SUCCEED) > 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); > + new_page = *hpage; > > /* > * Ensure we have slots for all the pages in the range. This is > @@ -2037,8 +2029,7 @@ static void khugepaged_scan_file(struct mm_struct *mm, struct file *file, > result = SCAN_EXCEED_NONE_PTE; > count_vm_event(THP_SCAN_EXCEED_NONE_PTE); > } else { > - node = khugepaged_find_target_node(cc); > - collapse_file(mm, file, start, hpage, node); > + collapse_file(mm, file, start, hpage, cc); > } > } > > -- > 2.36.1.255.ge46751e96f-goog >