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 91DBDC43334 for ; Mon, 6 Jun 2022 19:24:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 37D076B0074; Mon, 6 Jun 2022 15:24:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 32CE16B0075; Mon, 6 Jun 2022 15:24:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F5A66B0078; Mon, 6 Jun 2022 15:24:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 0873D6B0074 for ; Mon, 6 Jun 2022 15:24:25 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 0ACAA20BB5 for ; Mon, 6 Jun 2022 18:48:55 +0000 (UTC) X-FDA: 79548697830.07.4231FBF Received: from mail-qv1-f53.google.com (mail-qv1-f53.google.com [209.85.219.53]) by imf05.hostedemail.com (Postfix) with ESMTP id E97B010002A for ; Mon, 6 Jun 2022 18:48:15 +0000 (UTC) Received: by mail-qv1-f53.google.com with SMTP id l1so10827274qvh.1 for ; Mon, 06 Jun 2022 11:48:54 -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=nNhtiXpr2P8WJ6HeNjoYDl6XVq8ShSFFrrJ+5gLcDKY=; b=BcHWUcPVCoDRi5G0n2mDJo5IWNqm0WW2H7iRU2aXcnr8nF6tVqRQDOPe3/qNX4PCi7 1Df0Yo8YskSKQ5W/J6qDmlVSa2cxSci9lfrWifTZ5u06BdUf6bpvazNnnvN2yh8chpDe TpaARp/NI1v7rUHsdy3bmDtmVrdB086gdVCGtzBDYyx1Uk3jdJ7N1bf6JcKmHImq9mqk gYPtpPp8KNs1edp4vw+z7kNTh4hKjcphW+aLyr+0xvMXeXOrphHjf0iSWJ23ps0Vv178 L4DJb1jK0Hi/msB5pYmkynv6XJml+HqCyWpQkKpP/Kcqs4riZXrOVPsayHPKMJzKMawi PSkw== 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=nNhtiXpr2P8WJ6HeNjoYDl6XVq8ShSFFrrJ+5gLcDKY=; b=aWSYpUjgEh96TszCIIPvxnU2lONxQvHAAQJVykoHXzHgKaAlh8GAJi3BMnP9p4rwMQ 4FjUiAvJeq19SKg7JFuyOxhecrM9Ay2iI+cVI3IAJ3s4X+FNE4c0Vaxh10YfjGjs8cXy HJiIWPxSUcpa58w+yw06JIjDmO77SnVq6zsA7w2DK7FR76WejOL1Cm5JOxrG8YWKXIHk fjBL6PFMjGx6Nfe499HEET1o3eITYR+7rG/tBKgvfJByhCJ4iwHoI0HHdUKlHNBvOUB4 7aME/PZcfLKPg9GZwl0xRHf9y9ui44JMNaSNd+ZayyQrYRMfbxGShac2a354w4HEHtT3 s7Zw== X-Gm-Message-State: AOAM531BZwwPUgIM9Io38mQDI0CcJeD7vFmBot7Q+UvKsIyKhVzU663G mpR1UfFV5y/qkYbfB8ezOM5FDa2lx2dmE5kXVdlkqOQIh5o= X-Google-Smtp-Source: ABdhPJzzWpCA+ppl6pY1Z190yKQzsCpw7oY11PNIgIe8gKJ86n3tSQgfhdiXTFr9hb76g0TnbIaD53v+SLaNsGlxEDI= X-Received: by 2002:aa7:94a6:0:b0:51b:ddb8:1fcf with SMTP id a6-20020aa794a6000000b0051bddb81fcfmr19743598pfl.23.1654539921969; Mon, 06 Jun 2022 11:25:21 -0700 (PDT) MIME-Version: 1.0 References: <20220604004004.954674-1-zokeefe@google.com> <20220604004004.954674-2-zokeefe@google.com> In-Reply-To: <20220604004004.954674-2-zokeefe@google.com> From: Yang Shi Date: Mon, 6 Jun 2022 11:25:09 -0700 Message-ID: Subject: Re: [PATCH v6 01/15] mm: khugepaged: don't carry huge page to the next loop for !CONFIG_NUMA 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" X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: E97B010002A X-Stat-Signature: nkzmef4cgdwei5mtxg8pudbcofaizp6f X-Rspam-User: Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=BcHWUcPV; spf=pass (imf05.hostedemail.com: domain of shy828301@gmail.com designates 209.85.219.53 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1654541295-129815 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: > > From: Yang Shi > > The khugepaged has optimization to reduce huge page allocation calls for > !CONFIG_NUMA by carrying the allocated but failed to collapse huge page to > the next loop. CONFIG_NUMA doesn't do so since the next loop may try to > collapse huge page from a different node, so it doesn't make too much sense > to carry it. > > But when NUMA=n, the huge page is allocated by khugepaged_prealloc_page() > before scanning the address space, so it means huge page may be allocated > even though there is no suitable range for collapsing. Then the page would > be just freed if khugepaged already made enough progress. This could make > NUMA=n run have 5 times as much thp_collapse_alloc as NUMA=y run. This > problem actually makes things worse due to the way more pointless THP > allocations and makes the optimization pointless. > > This could be fixed by carrying the huge page across scans, but it will > complicate the code further and the huge page may be carried > indefinitely. But if we take one step back, the optimization itself seems > not worth keeping nowadays since: > * Not too many users build NUMA=n kernel nowadays even though the kernel is > actually running on a non-NUMA machine. Some small devices may run NUMA=n > kernel, but I don't think they actually use THP. > * Since commit 44042b449872 ("mm/page_alloc: allow high-order pages to be > stored on the per-cpu lists"), THP could be cached by pcp. This actually > somehow does the job done by the optimization. > > Cc: Hugh Dickins > Cc: "Kirill A. Shutemov" > > Signed-off-by: Zach O'Keefe Thanks for bringing the patch into the series. You could have my SOB on this patch. > --- > mm/khugepaged.c | 100 ++++++++---------------------------------------- > 1 file changed, 17 insertions(+), 83 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 476d79360101..cc3d6fb446d5 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -833,29 +833,30 @@ static int khugepaged_find_target_node(void) > last_khugepaged_target_node = target_node; > return target_node; > } > +#else > +static int khugepaged_find_target_node(void) > +{ > + return 0; > +} > +#endif > > -static bool khugepaged_prealloc_page(struct page **hpage, bool *wait) > +/* Sleep for the first alloc fail, break the loop for the second fail */ > +static bool alloc_fail_should_sleep(struct page **hpage, bool *wait) > { > if (IS_ERR(*hpage)) { > if (!*wait) > - return false; > + return true; > > *wait = false; > *hpage = NULL; > khugepaged_alloc_sleep(); > - } else if (*hpage) { > - put_page(*hpage); > - *hpage = NULL; > } > - > - return true; > + return false; > } > > static struct page * > khugepaged_alloc_page(struct page **hpage, gfp_t gfp, int node) > { > - VM_BUG_ON_PAGE(*hpage, *hpage); > - > *hpage = __alloc_pages_node(node, gfp, HPAGE_PMD_ORDER); > if (unlikely(!*hpage)) { > count_vm_event(THP_COLLAPSE_ALLOC_FAILED); > @@ -867,74 +868,6 @@ khugepaged_alloc_page(struct page **hpage, gfp_t gfp, int node) > count_vm_event(THP_COLLAPSE_ALLOC); > return *hpage; > } > -#else > -static int khugepaged_find_target_node(void) > -{ > - return 0; > -} > - > -static inline struct page *alloc_khugepaged_hugepage(void) > -{ > - struct page *page; > - > - page = alloc_pages(alloc_hugepage_khugepaged_gfpmask(), > - HPAGE_PMD_ORDER); > - if (page) > - prep_transhuge_page(page); > - return page; > -} > - > -static struct page *khugepaged_alloc_hugepage(bool *wait) > -{ > - struct page *hpage; > - > - do { > - hpage = alloc_khugepaged_hugepage(); > - if (!hpage) { > - count_vm_event(THP_COLLAPSE_ALLOC_FAILED); > - if (!*wait) > - return NULL; > - > - *wait = false; > - khugepaged_alloc_sleep(); > - } else > - count_vm_event(THP_COLLAPSE_ALLOC); > - } while (unlikely(!hpage) && likely(khugepaged_enabled())); > - > - return hpage; > -} > - > -static bool khugepaged_prealloc_page(struct page **hpage, bool *wait) > -{ > - /* > - * If the hpage allocated earlier was briefly exposed in page cache > - * before collapse_file() failed, it is possible that racing lookups > - * have not yet completed, and would then be unpleasantly surprised by > - * finding the hpage reused for the same mapping at a different offset. > - * Just release the previous allocation if there is any danger of that. > - */ > - if (*hpage && page_count(*hpage) > 1) { > - put_page(*hpage); > - *hpage = NULL; > - } > - > - if (!*hpage) > - *hpage = khugepaged_alloc_hugepage(wait); > - > - if (unlikely(!*hpage)) > - return false; > - > - return true; > -} > - > -static struct page * > -khugepaged_alloc_page(struct page **hpage, gfp_t gfp, int node) > -{ > - VM_BUG_ON(!*hpage); > - > - return *hpage; > -} > -#endif > > /* > * If mmap_lock temporarily dropped, revalidate vma > @@ -1188,8 +1121,10 @@ static void collapse_huge_page(struct mm_struct *mm, > out_up_write: > mmap_write_unlock(mm); > out_nolock: > - if (!IS_ERR_OR_NULL(*hpage)) > + if (!IS_ERR_OR_NULL(*hpage)) { > mem_cgroup_uncharge(page_folio(*hpage)); > + put_page(*hpage); > + } > trace_mm_collapse_huge_page(mm, isolated, result); > return; > } > @@ -1992,8 +1927,10 @@ static void collapse_file(struct mm_struct *mm, > unlock_page(new_page); > out: > VM_BUG_ON(!list_empty(&pagelist)); > - if (!IS_ERR_OR_NULL(*hpage)) > + if (!IS_ERR_OR_NULL(*hpage)) { > mem_cgroup_uncharge(page_folio(*hpage)); > + put_page(*hpage); > + } > /* TODO: tracepoints */ > } > > @@ -2243,7 +2180,7 @@ static void khugepaged_do_scan(void) > lru_add_drain_all(); > > while (progress < pages) { > - if (!khugepaged_prealloc_page(&hpage, &wait)) > + if (alloc_fail_should_sleep(&hpage, &wait)) > break; > > cond_resched(); > @@ -2262,9 +2199,6 @@ static void khugepaged_do_scan(void) > progress = pages; > spin_unlock(&khugepaged_mm_lock); > } > - > - if (!IS_ERR_OR_NULL(hpage)) > - put_page(hpage); > } > > static bool khugepaged_should_wakeup(void) > -- > 2.36.1.255.ge46751e96f-goog >