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 X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2404C4338F for ; Wed, 11 Aug 2021 22:25:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 23F766101E for ; Wed, 11 Aug 2021 22:25:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 23F766101E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 70BDE8D0001; Wed, 11 Aug 2021 18:25:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6BBF66B0071; Wed, 11 Aug 2021 18:25:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5AA038D0001; Wed, 11 Aug 2021 18:25:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0199.hostedemail.com [216.40.44.199]) by kanga.kvack.org (Postfix) with ESMTP id 4044C6B006C for ; Wed, 11 Aug 2021 18:25:34 -0400 (EDT) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C3DE9250BB for ; Wed, 11 Aug 2021 22:25:33 +0000 (UTC) X-FDA: 78464232546.08.54064D2 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf08.hostedemail.com (Postfix) with ESMTP id 79D0F3000259 for ; Wed, 11 Aug 2021 22:25:33 +0000 (UTC) Received: by mail-ed1-f46.google.com with SMTP id t1so6190997edd.6 for ; Wed, 11 Aug 2021 15:25:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rJpBS5OoiDFTcbVpGQiUJVnQBK3el7kqkKuLN5erzD4=; b=S5kxUee+y9SPPTHZlc8G3oed6PBjSOCQWPy0ktkl5FU0uFo2hlHwfM+FgM6fyYXTwW 0Et3JmqKvuLtXtazTkxCKshH4jf4Rpx8ygVPjCtpRAhr+5rt9HXDKSAh6d3SznjWVQiY E9ubEWX2uNubRFsGkty1uoCG2Z518FcQ/lrhUnH2upulPek42sFw9vOoPhZ597KLljvh piNYCCr7FHCXZfvMWosbXmRw8pjo6yfV5ys37OW2Evovf0ukAM2oALYCOnASzPO/xeO4 JD/8CJI4t6l3IvHAI5NJDnHWlhUIS0HsZ1dyfBRGBOdx9kCuOsWkcYojURsojWTOeHB6 8NCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rJpBS5OoiDFTcbVpGQiUJVnQBK3el7kqkKuLN5erzD4=; b=Ogat37ZRemiIpJqWZ+zUWijSIi4kr3csk0p69c2hcIy3yEqnKHh1IT3UdbLOz3KR8l tbIkNH7i4NrWKDS1DedNLFdy2DbakWOfEpJJ67SzI1RJlVEn0fy4vAy7cXrjiwtKAF3k R2XZIPTa1WDV07li0vUoVv/3tdTMfqw69TSx7V52EGiPfAqRCVHpm/6cy2nLBZS+H3mR lwmsjwZULZXA1vNshE/3FjZMHuLO4f4RuOCtnzMYAdBIjiLg71WVj32vq0KlYbcnBtTn pZOVLAssgiFXga9NMPjm9VIdeundO+hB60ymkBVSQ2xw9aE6T9kiW0kppMrRro2MUprP DO+w== X-Gm-Message-State: AOAM5314vVXWpoYtC+Cm3JGWHkuDcoTRVKs2un+S3Q1S7pwFPb7TxALo ali9ZsAyNP+G6zolDxiAYH1r6Q2uuXOs6Cb/aU4= X-Google-Smtp-Source: ABdhPJxCt0+TpHxvY37MRuA4EXyyS8X4JdVh9eV3T7FFVW0IJdg4EYKpXgdP2pTDIfNutmH0nIbJ5XxrPj/EIs/G6F8= X-Received: by 2002:aa7:d896:: with SMTP id u22mr1488054edq.290.1628720732183; Wed, 11 Aug 2021 15:25:32 -0700 (PDT) MIME-Version: 1.0 References: <20210731063938.1391602-1-yuzhao@google.com> <20210731063938.1391602-3-yuzhao@google.com> In-Reply-To: From: Yang Shi Date: Wed, 11 Aug 2021 15:25:19 -0700 Message-ID: Subject: Re: [PATCH 2/3] mm: free zapped tail pages when splitting isolated thp To: Yu Zhao Cc: Linux MM , Andrew Morton , Hugh Dickins , "Kirill A . Shutemov" , Matthew Wilcox , Vlastimil Babka , Zi Yan , Linux Kernel Mailing List , Shuang Zhai Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 79D0F3000259 Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=S5kxUee+; spf=pass (imf08.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam01 X-Stat-Signature: 3q8cgcci7jqb5eueka1heec3kwi8noh1 X-HE-Tag: 1628720733-355117 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 Sun, Aug 8, 2021 at 10:49 AM Yu Zhao wrote: > > On Wed, Aug 4, 2021 at 6:13 PM Yang Shi wrote: > > > > On Fri, Jul 30, 2021 at 11:39 PM Yu Zhao wrote: > > > > > > If a tail page has only two references left, one inherited from the > > > isolation of its head and the other from lru_add_page_tail() which we > > > are about to drop, it means this tail page was concurrently zapped. > > > Then we can safely free it and save page reclaim or migration the > > > trouble of trying it. > > > > > > Signed-off-by: Yu Zhao > > > Tested-by: Shuang Zhai > > > --- > > > include/linux/vm_event_item.h | 1 + > > > mm/huge_memory.c | 28 ++++++++++++++++++++++++++++ > > > mm/vmstat.c | 1 + > > > 3 files changed, 30 insertions(+) > > > > > > diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h > > > index ae0dd1948c2b..829eeac84094 100644 > > > --- a/include/linux/vm_event_item.h > > > +++ b/include/linux/vm_event_item.h > > > @@ -99,6 +99,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT, > > > #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > > THP_SPLIT_PUD, > > > #endif > > > + THP_SPLIT_FREE, > > > THP_ZERO_PAGE_ALLOC, > > > THP_ZERO_PAGE_ALLOC_FAILED, > > > THP_SWPOUT, > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > > index d8b655856e79..5120478bca41 100644 > > > --- a/mm/huge_memory.c > > > +++ b/mm/huge_memory.c > > > @@ -2432,6 +2432,8 @@ static void __split_huge_page(struct page *page, struct list_head *list, > > > struct address_space *swap_cache = NULL; > > > unsigned long offset = 0; > > > unsigned int nr = thp_nr_pages(head); > > > + LIST_HEAD(pages_to_free); > > > + int nr_pages_to_free = 0; > > > int i; > > > > > > VM_BUG_ON_PAGE(list && PageLRU(head), head); > > > @@ -2506,6 +2508,25 @@ static void __split_huge_page(struct page *page, struct list_head *list, > > > continue; > > > unlock_page(subpage); > > > > > > + /* > > > + * If a tail page has only two references left, one inherited > > > + * from the isolation of its head and the other from > > > + * lru_add_page_tail() which we are about to drop, it means this > > > + * tail page was concurrently zapped. Then we can safely free it > > > + * and save page reclaim or migration the trouble of trying it. > > > + */ > > > + if (list && page_ref_freeze(subpage, 2)) { > > > + VM_BUG_ON_PAGE(PageLRU(subpage), subpage); > > > + VM_BUG_ON_PAGE(PageCompound(subpage), subpage); > > > + VM_BUG_ON_PAGE(page_mapped(subpage), subpage); > > > + > > > + ClearPageActive(subpage); > > > + ClearPageUnevictable(subpage); > > > + list_move(&subpage->lru, &pages_to_free); > > > + nr_pages_to_free++; > > > + continue; > > > + } > > > > Yes, such page could be freed instead of swapping out. But I'm > > wondering if we could have some simpler implementation. Since such > > pages will be re-added to page list, so we should be able to check > > their refcount in shrink_page_list(). If the refcount is 1, the > > refcount inc'ed by lru_add_page_tail() has been put by later > > put_page(), we know it is freed under us since the only refcount comes > > from isolation, we could just jump to "keep" (the label in > > shrink_page_list()), then such page will be freed later by > > shrink_inactive_list(). > > > > For MADV_PAGEOUT, I think we could add some logic to handle such page > > after shrink_page_list(), just like what shrink_inactive_list() does. > > > > Migration already handles refcount == 1 page, so should not need any change. > > > > Is this idea feasible? > > Yes, but then we would have to loop over the tail pages twice, here > and in shrink_page_list(), right? I don't quite get what you mean "loop over the tail pages twice". Once THP is isolated then get split, all the tail pages will be put on the list (local list for isolated pages), then the reclaimer would deal with the head page, then continue to iterate the list to deal with tail pages. Your patch could free the tail pages earlier. But it should not make too much difference to free the tail pages a little bit later IMHO. > > In addition, if we try to freeze the refcount of a page in > shrink_page_list(), we couldn't be certain whether this page used to > be a tail page. So we would have to test every page. If a page wasn't > a tail page, it's unlikely for its refcount to drop unless there is a > race. But this patch isn't really intended to optimize such a race. > It's mainly for the next, i.e., we know there is a good chance to drop > tail pages (~10% on our systems). Sounds reasonable? Thanks. I'm not sure what is the main source of the partial mapped THPs from your fleets. But if most of them are generated by MADV_DONTNEED (this is used by some userspace memory allocator libs), they should be on deferred split list too. Currently deferred split shrinker just shrinks those THPs (simply split them and free unmapped sub pages) proportionally, we definitely could shrink them more aggressively, for example, by setting shrinker->seeks to 0. I'm wondering if this will achieve a similar effect or not. I really don't have any objection to free such pages, but just wondering if we could have something simpler or not. > > > > + > > > /* > > > * Subpages may be freed if there wasn't any mapping > > > * like if add_to_swap() is running on a lru page that > > > @@ -2515,6 +2536,13 @@ static void __split_huge_page(struct page *page, struct list_head *list, > > > */ > > > put_page(subpage); > > > } > > > + > > > + if (!nr_pages_to_free) > > > + return; > > > + > > > + mem_cgroup_uncharge_list(&pages_to_free); > > > + free_unref_page_list(&pages_to_free); > > > + count_vm_events(THP_SPLIT_FREE, nr_pages_to_free); > > > } > > > > > > int total_mapcount(struct page *page) > > > diff --git a/mm/vmstat.c b/mm/vmstat.c > > > index b0534e068166..f486e5d98d96 100644 > > > --- a/mm/vmstat.c > > > +++ b/mm/vmstat.c > > > @@ -1300,6 +1300,7 @@ const char * const vmstat_text[] = { > > > #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > > "thp_split_pud", > > > #endif > > > + "thp_split_free", > > > "thp_zero_page_alloc", > > > "thp_zero_page_alloc_failed", > > > "thp_swpout", > > > -- > > > 2.32.0.554.ge1b32706d8-goog > > >