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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL 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 0C367C63798 for ; Wed, 25 Nov 2020 15:30:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 69695208CA for ; Wed, 25 Nov 2020 15:30:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="WhZUTC9Y" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 69695208CA Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D1AAB6B0080; Wed, 25 Nov 2020 10:30:20 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CCACB6B0082; Wed, 25 Nov 2020 10:30:20 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B43216B0083; Wed, 25 Nov 2020 10:30:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0048.hostedemail.com [216.40.44.48]) by kanga.kvack.org (Postfix) with ESMTP id 985EB6B0080 for ; Wed, 25 Nov 2020 10:30:20 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 5B2AD181AEF2A for ; Wed, 25 Nov 2020 15:30:20 +0000 (UTC) X-FDA: 77523327000.03.bells15_4f0493327377 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin03.hostedemail.com (Postfix) with ESMTP id 2048428A4EA for ; Wed, 25 Nov 2020 15:30:20 +0000 (UTC) X-HE-Tag: bells15_4f0493327377 X-Filterd-Recvd-Size: 11405 Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) by imf50.hostedemail.com (Postfix) with ESMTP for ; Wed, 25 Nov 2020 15:30:19 +0000 (UTC) Received: by mail-lj1-f196.google.com with SMTP id j10so2725020lja.5 for ; Wed, 25 Nov 2020 07:30:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NeZYpN6ceKaX6TfXbLKbNUGfPhPG7kooiv8AQejhsJc=; b=WhZUTC9YUIpB9RDGNZahAcAiuaU/CI786AWFeNIHLbPBoula2nT2RK6gxq+nfadkvL pFSQJPfM/4O3e3tKuWy/F9N7WdoRF7LefVP72CYJyIKtGcsLF4sfLwhFyxpyaTZhekXY MXqXcL81CXOzVPL7JEXOGBeWNCp5wBkHsnJHltA+/2maDrr7wLAx2vVnxQDthFdGR7+Y yRGt/yn+rOIx3A5YSdCQcKQxbSxvAa2ESV5ensOS+NvmH9Ugdj7inz+skWD88KmFqu6J squmRbQRngpm+1VF0HT29MGioKrdZIqV7NzUEYdHGTDfXPVp/ZJfZLL/8ak+cqMBKjKS 8ySg== 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=NeZYpN6ceKaX6TfXbLKbNUGfPhPG7kooiv8AQejhsJc=; b=P1tyjUYovqYhUP7voVJkN5BK6p8+GgHsnFbYuW/BX46/qwFYmd/HMe1U4ekH6ZOpsN FGSTY0oWeCWhEZnKvC3QZxyI6egYi+TMAYM39Cmrz5K+MhzvPYHhRoMCSkMCpP6TUTaQ F6eOHPGcN9RDOGCh91MLmho7xGQAhQky1D608bQqwTtN2Z5msroRI2XOcL6+pN4C3mA4 lmmu8LfdJmSz1PXF+2LmmqTo+6EeddmuPDidLlhtpCWbAkpKLjzsXMAvr6HEHMQ309sV YoUAlz0eILYAaRKwiTY5gSsvzRsPxu/FugBr4W5DZENO820uto/Ece71O1jX50l0bBqb RneA== X-Gm-Message-State: AOAM530rQ/KyLaHDNLk3ED1Y5d28HNH/WNYkOX0GNvEgzCax66tXdHEN Hu34A1zHlbUPKJfEKPYoyFqewm0il4wzYuSjVBuTFw== X-Google-Smtp-Source: ABdhPJyCnvynWTodcwSYTNdjI7u8KaDsboYP5Z/mWviR481Shbyl8sfXiINAWblDR2UGCQGwq2zitVZfNFgmYGIwaOM= X-Received: by 2002:a2e:b16a:: with SMTP id a10mr1650615ljm.446.1606318217780; Wed, 25 Nov 2020 07:30:17 -0800 (PST) MIME-Version: 1.0 References: <20201020085940.13875-1-sjpark@amazon.com> <20201020085940.13875-6-sjpark@amazon.com> In-Reply-To: <20201020085940.13875-6-sjpark@amazon.com> From: Shakeel Butt Date: Wed, 25 Nov 2020 07:30:06 -0800 Message-ID: Subject: Re: [PATCH v22 05/18] mm/idle_page_tracking: Make PG_(idle|young) reusable To: SeongJae Park Cc: Andrew Morton , SeongJae Park , Jonathan.Cameron@huawei.com, Andrea Arcangeli , acme@kernel.org, alexander.shishkin@linux.intel.com, amit@kernel.org, benh@kernel.crashing.org, brendan.d.gregg@gmail.com, Brendan Higgins , Qian Cai , Colin Ian King , Jonathan Corbet , David Hildenbrand , dwmw@amazon.com, Marco Elver , "Du, Fan" , foersleo@amazon.de, Greg Thelen , Ian Rogers , jolsa@redhat.com, "Kirill A. Shutemov" , Mark Rutland , Mel Gorman , Minchan Kim , Ingo Molnar , namhyung@kernel.org, "Peter Zijlstra (Intel)" , Randy Dunlap , Rik van Riel , David Rientjes , Steven Rostedt , Mike Rapoport , sblbir@amazon.com, Shuah Khan , sj38.park@gmail.com, snu@amazon.de, Vlastimil Babka , Vladimir Davydov , Yang Shi , Huang Ying , zgf574564920@gmail.com, linux-damon@amazon.com, Linux MM , linux-doc@vger.kernel.org, LKML Content-Type: text/plain; charset="UTF-8" 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 Tue, Oct 20, 2020 at 2:04 AM SeongJae Park wrote: > > From: SeongJae Park > > PG_idle and PG_young allows the two PTE Accessed bit users, > IDLE_PAGE_TRACKING and the reclaim logic concurrently work while don't > interfere each other. That is, when they need to clear the Accessed > bit, they set PG_young Only PG_young bit > and PG_idle to represent the previous state of > the bit, respectively. And when they need to read the bit, if the bit > is cleared, they further read the PG_young Again only PG_young bit. PG_idle bit is only read (and set) by the page idle tracking code and it can be cleared by others (reclaim or file access). > and PG_idle, respectively, to > know whether the other has cleared the bit meanwhile or not. > > We could add another page flag and extend the mechanism to use the flag > if we need to add another concurrent PTE Accessed bit user subsystem. > However, it would be only waste the space. Instead, if the new > subsystem is mutually exclusive with IDLE_PAGE_TRACKING, it could simply > reuse the PG_idle flag. However, it's impossible because the flags are > dependent on IDLE_PAGE_TRACKING. > > To allow such reuse of the flags, this commit separates the PG_young and > PG_idle flag logic from IDLE_PAGE_TRACKING and introduces new kernel > config, 'PAGE_IDLE_FLAG'. Hence, if !IDLE_PAGE_TRACKING and > IDLE_PAGE_FLAG, a new subsystem would be able to reuse PG_idle. > > In the next commit, DAMON's reference implementation of the virtual > memory address space monitoring primitives will use it. > > Signed-off-by: SeongJae Park > --- > include/linux/page-flags.h | 4 ++-- > include/linux/page_ext.h | 2 +- > include/linux/page_idle.h | 6 +++--- > include/trace/events/mmflags.h | 2 +- > mm/Kconfig | 8 ++++++++ > mm/page_ext.c | 12 +++++++++++- > mm/page_idle.c | 10 ---------- > 7 files changed, 26 insertions(+), 18 deletions(-) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index 6be1aa559b1e..7736d290bb61 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -132,7 +132,7 @@ enum pageflags { > #ifdef CONFIG_MEMORY_FAILURE > PG_hwpoison, /* hardware poisoned page. Don't touch */ > #endif > -#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT) > +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT) > PG_young, > PG_idle, > #endif > @@ -432,7 +432,7 @@ static inline bool set_hwpoison_free_buddy_page(struct page *page) > #define __PG_HWPOISON 0 > #endif > > -#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT) > +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT) > TESTPAGEFLAG(Young, young, PF_ANY) > SETPAGEFLAG(Young, young, PF_ANY) > TESTCLEARFLAG(Young, young, PF_ANY) > diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h > index cfce186f0c4e..c9cbc9756011 100644 > --- a/include/linux/page_ext.h > +++ b/include/linux/page_ext.h > @@ -19,7 +19,7 @@ struct page_ext_operations { > enum page_ext_flags { > PAGE_EXT_OWNER, > PAGE_EXT_OWNER_ALLOCATED, > -#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT) > +#if defined(CONFIG_PAGE_IDLE_FLAG) && !defined(CONFIG_64BIT) > PAGE_EXT_YOUNG, > PAGE_EXT_IDLE, > #endif > diff --git a/include/linux/page_idle.h b/include/linux/page_idle.h > index 1e894d34bdce..d8a6aecf99cb 100644 > --- a/include/linux/page_idle.h > +++ b/include/linux/page_idle.h > @@ -6,7 +6,7 @@ > #include > #include > > -#ifdef CONFIG_IDLE_PAGE_TRACKING > +#ifdef CONFIG_PAGE_IDLE_FLAG > > #ifdef CONFIG_64BIT > static inline bool page_is_young(struct page *page) > @@ -106,7 +106,7 @@ static inline void clear_page_idle(struct page *page) > } > #endif /* CONFIG_64BIT */ > > -#else /* !CONFIG_IDLE_PAGE_TRACKING */ > +#else /* !CONFIG_PAGE_IDLE_FLAG */ > > static inline bool page_is_young(struct page *page) > { > @@ -135,6 +135,6 @@ static inline void clear_page_idle(struct page *page) > { > } > > -#endif /* CONFIG_IDLE_PAGE_TRACKING */ > +#endif /* CONFIG_PAGE_IDLE_FLAG */ > > #endif /* _LINUX_MM_PAGE_IDLE_H */ > diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h > index 5fb752034386..4d182c32071b 100644 > --- a/include/trace/events/mmflags.h > +++ b/include/trace/events/mmflags.h > @@ -73,7 +73,7 @@ > #define IF_HAVE_PG_HWPOISON(flag,string) > #endif > > -#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT) > +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT) > #define IF_HAVE_PG_IDLE(flag,string) ,{1UL << flag, string} > #else > #define IF_HAVE_PG_IDLE(flag,string) > diff --git a/mm/Kconfig b/mm/Kconfig > index 19fe2251c87a..044317ef9143 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -761,10 +761,18 @@ config DEFERRED_STRUCT_PAGE_INIT > lifetime of the system until these kthreads finish the > initialisation. > > +config PAGE_IDLE_FLAG > + bool "Add PG_idle and PG_young flags" > + help > + This feature adds PG_idle and PG_young flags in 'struct page'. PTE > + Accessed bit writers can set the state of the bit in the flags to let > + other PTE Accessed bit readers don't disturbed. > + > config IDLE_PAGE_TRACKING > bool "Enable idle page tracking" > depends on SYSFS && MMU > select PAGE_EXTENSION if !64BIT > + select PAGE_IDLE_FLAG > help > This feature allows to estimate the amount of user pages that have > not been touched during a given period of time. This information can > diff --git a/mm/page_ext.c b/mm/page_ext.c > index a3616f7a0e9e..f9a6ff65ac0a 100644 > --- a/mm/page_ext.c > +++ b/mm/page_ext.c > @@ -58,11 +58,21 @@ > * can utilize this callback to initialize the state of it correctly. > */ > Is there a need to move the following code in this patch? > +#if defined(CONFIG_PAGE_IDLE_FLAG) && !defined(CONFIG_64BIT) > +static bool need_page_idle(void) > +{ > + return true; > +} > +struct page_ext_operations page_idle_ops = { > + .need = need_page_idle, > +}; > +#endif > + > static struct page_ext_operations *page_ext_ops[] = { > #ifdef CONFIG_PAGE_OWNER > &page_owner_ops, > #endif > -#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT) > +#if defined(CONFIG_PAGE_IDLE_FLAG) && !defined(CONFIG_64BIT) > &page_idle_ops, > #endif > }; > diff --git a/mm/page_idle.c b/mm/page_idle.c > index 057c61df12db..144fb4ed961d 100644 > --- a/mm/page_idle.c > +++ b/mm/page_idle.c > @@ -211,16 +211,6 @@ static const struct attribute_group page_idle_attr_group = { > .name = "page_idle", > }; > > -#ifndef CONFIG_64BIT > -static bool need_page_idle(void) > -{ > - return true; > -} > -struct page_ext_operations page_idle_ops = { > - .need = need_page_idle, > -}; > -#endif > - > static int __init page_idle_init(void) > { > int err; > -- > 2.17.1 > Overall this patch looks good to me.