| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALvZod5DGLtegPdDjj72WOO1RmR1MV_8DE+NEakg1PYGurHNUQ@mail.gmail.com>
Date: Wed, 25 Nov 2020 07:30:06 -0800
From: Shakeel Butt <shakeelb@...gle.com>
To: SeongJae Park <sjpark@...zon.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
SeongJae Park <sjpark@...zon.de>, Jonathan.Cameron@...wei.com,
Andrea Arcangeli <aarcange@...hat.com>, acme@...nel.org,
alexander.shishkin@...ux.intel.com, amit@...nel.org,
benh@...nel.crashing.org, brendan.d.gregg@...il.com,
Brendan Higgins <brendanhiggins@...gle.com>,
Qian Cai <cai@....pw>,
Colin Ian King <colin.king@...onical.com>,
Jonathan Corbet <corbet@....net>,
David Hildenbrand <david@...hat.com>, dwmw@...zon.com,
Marco Elver <elver@...gle.com>, "Du, Fan" <fan.du@...el.com>,
foersleo@...zon.de, Greg Thelen <gthelen@...gle.com>,
Ian Rogers <irogers@...gle.com>, jolsa@...hat.com,
"Kirill A. Shutemov" <kirill@...temov.name>,
Mark Rutland <mark.rutland@....com>,
Mel Gorman <mgorman@...e.de>, Minchan Kim <minchan@...nel.org>,
Ingo Molnar <mingo@...hat.com>, namhyung@...nel.org,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Randy Dunlap <rdunlap@...radead.org>,
Rik van Riel <riel@...riel.com>,
David Rientjes <rientjes@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>,
Mike Rapoport <rppt@...nel.org>, sblbir@...zon.com,
Shuah Khan <shuah@...nel.org>, sj38.park@...il.com,
snu@...zon.de, Vlastimil Babka <vbabka@...e.cz>,
Vladimir Davydov <vdavydov.dev@...il.com>,
Yang Shi <yang.shi@...ux.alibaba.com>,
Huang Ying <ying.huang@...el.com>, zgf574564920@...il.com,
linux-damon@...zon.com, Linux MM <linux-mm@...ck.org>,
linux-doc@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v22 05/18] mm/idle_page_tracking: Make PG_(idle|young) reusable
On Tue, Oct 20, 2020 at 2:04 AM SeongJae Park <sjpark@...zon.com> wrote:
>
> From: SeongJae Park <sjpark@...zon.de>
>
> 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 <sjpark@...zon.de>
> ---
> 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 <linux/page-flags.h>
> #include <linux/page_ext.h>
>
> -#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.
Powered by blists - more mailing lists