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=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 845F9C18E7A for ; Mon, 9 Mar 2020 09:15:57 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3E06D20674 for ; Mon, 9 Mar 2020 09:15:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3E06D20674 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C6E856B0003; Mon, 9 Mar 2020 05:15:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C1F686B0006; Mon, 9 Mar 2020 05:15:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B35C26B0007; Mon, 9 Mar 2020 05:15:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0061.hostedemail.com [216.40.44.61]) by kanga.kvack.org (Postfix) with ESMTP id 9B05A6B0003 for ; Mon, 9 Mar 2020 05:15:56 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 56293181AC9BF for ; Mon, 9 Mar 2020 09:15:56 +0000 (UTC) X-FDA: 76575266712.07.twist38_33c0ca1ec7001 X-HE-Tag: twist38_33c0ca1ec7001 X-Filterd-Recvd-Size: 9764 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by imf08.hostedemail.com (Postfix) with ESMTP for ; Mon, 9 Mar 2020 09:15:55 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Mar 2020 02:15:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,532,1574150400"; d="scan'208";a="276440931" Received: from yhuang-dev.sh.intel.com (HELO yhuang-dev) ([10.239.159.23]) by fmsmga002.fm.intel.com with ESMTP; 09 Mar 2020 02:15:50 -0700 From: "Huang\, Ying" To: David Hildenbrand Cc: Andrew Morton , , , Dave Hansen , Mel Gorman , Vlastimil Babka , Zi Yan , Michal Hocko , Peter Zijlstra , Minchan Kim , Johannes Weiner , Hugh Dickins Subject: Re: [PATCH -V3] mm: Add PageLayzyFree() helper functions for MADV_FREE References: <20200309021744.1309482-1-ying.huang@intel.com> <68360241-eb18-b3d8-bf6f-4dbbed258ee6@redhat.com> Date: Mon, 09 Mar 2020 17:15:50 +0800 In-Reply-To: <68360241-eb18-b3d8-bf6f-4dbbed258ee6@redhat.com> (David Hildenbrand's message of "Mon, 9 Mar 2020 09:55:38 +0100") Message-ID: <87r1y1yjll.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii 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: David Hildenbrand writes: > On 09.03.20 03:17, Huang, Ying wrote: >> From: Huang Ying >> >> Now PageSwapBacked() is used as the helper function to check whether >> pages have been freed lazily via MADV_FREE. This isn't very obvious. >> So Dave suggested to add PageLazyFree() family helper functions to >> improve the code readability. >> >> Signed-off-by: "Huang, Ying" >> Suggested-by: Dave Hansen >> Cc: David Hildenbrand >> Cc: Mel Gorman >> Cc: Vlastimil Babka >> Cc: Zi Yan >> Cc: Michal Hocko >> Cc: Peter Zijlstra >> Cc: Minchan Kim >> Cc: Johannes Weiner >> Cc: Hugh Dickins >> --- >> Changelog: >> >> v3: >> >> - Improved comments, Thanks David Hildenbrand! >> >> - Use the helper function in /proc/PID/smaps lazyfree reporting. >> >> v2: >> >> - Avoid code bloat via removing VM_BUG_ON_PAGE(), which doesn't exist >> in the original code. Now there is no any text/data/bss size >> change. >> >> - Fix one wrong replacement in try_to_unmap_one(). >> >> --- >> fs/proc/task_mmu.c | 2 +- >> include/linux/page-flags.h | 25 +++++++++++++++++++++++++ >> mm/rmap.c | 4 ++-- >> mm/swap.c | 11 +++-------- >> mm/vmscan.c | 7 +++---- >> 5 files changed, 34 insertions(+), 15 deletions(-) >> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c >> index 3ba9ae83bff5..3458d5711e57 100644 >> --- a/fs/proc/task_mmu.c >> +++ b/fs/proc/task_mmu.c >> @@ -471,7 +471,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page, >> */ >> if (PageAnon(page)) { >> mss->anonymous += size; >> - if (!PageSwapBacked(page) && !dirty && !PageDirty(page)) >> + if (__PageLazyFree(page) && !dirty && !PageDirty(page)) >> mss->lazyfree += size; >> } >> >> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h >> index 49c2697046b9..bb26f74cbe8e 100644 >> --- a/include/linux/page-flags.h >> +++ b/include/linux/page-flags.h >> @@ -498,6 +498,31 @@ static __always_inline int PageKsm(struct page *page) >> TESTPAGEFLAG_FALSE(Ksm) >> #endif >> >> +/* >> + * For pages freed lazily via MADV_FREE. Lazyfree pages are clean >> + * anonymous pages. They don't have PG_swapbacked set, to distinguish >> + * them from normal anonymous pages. >> + */ >> +static __always_inline int __PageLazyFree(struct page *page) >> +{ >> + return !PageSwapBacked(page); >> +} >> + >> +static __always_inline int PageLazyFree(struct page *page) >> +{ >> + return PageAnon(page) && __PageLazyFree(page); >> +} >> + >> +static __always_inline void SetPageLazyFree(struct page *page) >> +{ >> + ClearPageSwapBacked(page); >> +} >> + >> +static __always_inline void ClearPageLazyFree(struct page *page) >> +{ >> + SetPageSwapBacked(page); >> +} >> + >> u64 stable_page_flags(struct page *page); >> >> static inline int PageUptodate(struct page *page) >> diff --git a/mm/rmap.c b/mm/rmap.c >> index 1c02adaa233e..6ec96c8e7826 100644 >> --- a/mm/rmap.c >> +++ b/mm/rmap.c >> @@ -1609,7 +1609,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma, >> } >> >> /* MADV_FREE page check */ >> - if (!PageSwapBacked(page)) { >> + if (__PageLazyFree(page)) { >> if (!PageDirty(page)) { >> /* Invalidate as we cleared the pte */ >> mmu_notifier_invalidate_range(mm, >> @@ -1623,7 +1623,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma, >> * discarded. Remap the page to page table. >> */ >> set_pte_at(mm, address, pvmw.pte, pteval); >> - SetPageSwapBacked(page); >> + ClearPageLazyFree(page); >> ret = false; >> page_vma_mapped_walk_done(&pvmw); >> break; >> diff --git a/mm/swap.c b/mm/swap.c >> index c1d3ca80ea10..d83f2cd4cdb8 100644 >> --- a/mm/swap.c >> +++ b/mm/swap.c >> @@ -563,7 +563,7 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec, >> static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec, >> void *arg) >> { >> - if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) && >> + if (PageLRU(page) && PageAnon(page) && !__PageLazyFree(page) && >> !PageSwapCache(page) && !PageUnevictable(page)) { >> bool active = PageActive(page); >> >> @@ -571,12 +571,7 @@ static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec, >> LRU_INACTIVE_ANON + active); >> ClearPageActive(page); >> ClearPageReferenced(page); >> - /* >> - * lazyfree pages are clean anonymous pages. They have >> - * SwapBacked flag cleared to distinguish normal anonymous >> - * pages >> - */ >> - ClearPageSwapBacked(page); >> + SetPageLazyFree(page); >> add_page_to_lru_list(page, lruvec, LRU_INACTIVE_FILE); >> >> __count_vm_events(PGLAZYFREE, hpage_nr_pages(page)); >> @@ -678,7 +673,7 @@ void deactivate_page(struct page *page) >> */ >> void mark_page_lazyfree(struct page *page) >> { >> - if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) && >> + if (PageLRU(page) && PageAnon(page) && !__PageLazyFree(page) && >> !PageSwapCache(page) && !PageUnevictable(page)) { >> struct pagevec *pvec = &get_cpu_var(lru_lazyfree_pvecs); >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index eca49a1c2f68..40bb41ada2d2 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1043,8 +1043,7 @@ static void page_check_dirty_writeback(struct page *page, >> * Anonymous pages are not handled by flushers and must be written >> * from reclaim context. Do not stall reclaim based on them >> */ >> - if (!page_is_file_cache(page) || >> - (PageAnon(page) && !PageSwapBacked(page))) { >> + if (!page_is_file_cache(page) || PageLazyFree(page)) { >> *dirty = false; >> *writeback = false; >> return; >> @@ -1235,7 +1234,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, >> * Try to allocate it some swap space here. >> * Lazyfree page could be freed directly >> */ >> - if (PageAnon(page) && PageSwapBacked(page)) { >> + if (PageAnon(page) && !__PageLazyFree(page)) { >> if (!PageSwapCache(page)) { >> if (!(sc->gfp_mask & __GFP_IO)) >> goto keep_locked; >> @@ -1411,7 +1410,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, >> } >> } >> >> - if (PageAnon(page) && !PageSwapBacked(page)) { >> + if (PageLazyFree(page)) { >> /* follow __remove_mapping for reference */ >> if (!page_ref_freeze(page, 1)) >> goto keep_locked; >> > > I still prefer something like > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index fd6d4670ccc3..7538501230bd 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -63,6 +63,10 @@ > * page_waitqueue(page) is a wait queue of all tasks waiting for the page > * to become unlocked. > * > + * PG_swapbacked used with anonymous pages (PageAnon()) indicates that a > + * page is backed by swap. Anonymous pages without PG_swapbacked are > + * pages that can be lazily freed (e.g., MADV_FREE) on demand. > + * > * PG_uptodate tells whether the page's contents is valid. When a read > * completes, the page becomes uptodate, unless a disk I/O error happened. > * Why not just send a formal patch? So Andrew can just pick anything he likes. I am totally OK with that. > and really don't like the use of !__PageLazyFree() instead of PageSwapBacked(). If adopted, !__PageLazyFree() should only be used in the context where we really want to check whether pages are freed lazily. Otherwise, PageSwapBacked() should be used. Best Regards, Huang, Ying