On Tue, Oct 20, 2020 at 2:04 AM SeongJae Park <sjp...@amazon.com> wrote:
>
> From: SeongJae Park <sjp...@amazon.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 <sjp...@amazon.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.
