In this commit:
commit fedc37448fb1be5d03e420ca7791d4286893d5ec
Author: SeongJae Park <[email protected]>
Date: Tue Aug 10 16:55:51 2021 +1000
mm/idle_page_tracking: make PG_idle reusable
diff --git a/mm/Kconfig b/mm/Kconfig
index 504336de9a1e..d0b85dc12429 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -739,10 +739,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.
This needs to be converted to proper, or at least comprehensible, English....
From: SeongJae Park <[email protected]>
Hello Valdis,
On Thu, 12 Aug 2021 05:21:57 -0400 "Valdis =?utf-8?Q?Kl=c4=93tnieks?=" <[email protected]> wrote:
> In this commit:
>
> commit fedc37448fb1be5d03e420ca7791d4286893d5ec
> Author: SeongJae Park <[email protected]>
> Date: Tue Aug 10 16:55:51 2021 +1000
>
> mm/idle_page_tracking: make PG_idle reusable
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 504336de9a1e..d0b85dc12429 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -739,10 +739,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.
>
> This needs to be converted to proper, or at least comprehensible, English....
Thank you for the comment.
How about below?
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -743,9 +743,9 @@ config PAGE_IDLE_FLAG
bool "Add PG_idle and PG_young flags"
select PAGE_EXTENSION if !64BIT
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.
+ This feature adds 'PG_idle' and 'PG_young' flags in 'struct page'.
+ PTE Accessed bit writers can save the state of the bit in the flags
+ to let other PTE Accessed bit readers don't get disturbed.
Thanks,
SeongJae Park
On Thu, 12 Aug 2021 09:42:40 -0000, SeongJae Park said:
> - 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.
> + This feature adds 'PG_idle' and 'PG_young' flags in 'struct page'.
> + PTE Accessed bit writers can save the state of the bit in the flags
> + to let other PTE Accessed bit readers don't get disturbed.
Well, better English would be "to let other ... not be disturbed'.
But I was rather hoping for an explanation of what "don't get disturbed" actually means.
If you are "save the state of the bit", are you saving the *previous* value (in
which case, other readers of the bit may or may not encounter changed behavior),
or are you saving a shadow copy that may have different values than the original
flags, and only used by a few routines?
Or are you creating two new status flags that are only used by several
optimized/fastpath routines and ignored by the other readers of the various
flag bits?
So a better description would be something like
This feature adds two new status bits PG_idle and PG_young to 'struct page'.
This allows passing additional information to certain users of PTE Accessed so
they can use an optimized codepath bypassing expensive checks for certain
common cases.
or "so they can provide <describe different behavior>"
or whatever this option is doing.
On Thu, 12 Aug 2021 09:42:40 +0000 SeongJae Park <[email protected]> wrote:
> > +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.
> >
> > This needs to be converted to proper, or at least comprehensible, English....
>
> Thank you for the comment.
>
> How about below?
>
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -743,9 +743,9 @@ config PAGE_IDLE_FLAG
> bool "Add PG_idle and PG_young flags"
> select PAGE_EXTENSION if !64BIT
> 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.
> + This feature adds 'PG_idle' and 'PG_young' flags in 'struct page'.
> + PTE Accessed bit writers can save the state of the bit in the flags
> + to let other PTE Accessed bit readers don't get disturbed.
How about this?
--- a/mm/Kconfig~mm-idle_page_tracking-make-pg_idle-reusable-fix-fix
+++ a/mm/Kconfig
@@ -743,9 +743,9 @@ config PAGE_IDLE_FLAG
bool "Add PG_idle and PG_young flags"
select PAGE_EXTENSION if !64BIT
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.
+ This adds PG_idle and PG_young flags to 'struct page'. PTE Accessed
+ bit writers can set the state of the bit in the flags so that PTE
+ Accessed bit readers may avoid disturbance.
config IDLE_PAGE_TRACKING
bool "Enable idle page tracking"
Also, is there any way in which we can avoid presenting this option to
the user? Because most users will have real trouble understanding what
this thing is for. Can we simply select it when needed, as dictated by
other, higher-level config options?
From: SeongJae Park <[email protected]>
On Thu, 12 Aug 2021 13:19:21 -0700 Andrew Morton <[email protected]> wrote:
> On Thu, 12 Aug 2021 09:42:40 +0000 SeongJae Park <[email protected]> wrote:
>
> > > +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.
> > >
> > > This needs to be converted to proper, or at least comprehensible, English....
> >
> > Thank you for the comment.
> >
> > How about below?
> >
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -743,9 +743,9 @@ config PAGE_IDLE_FLAG
> > bool "Add PG_idle and PG_young flags"
> > select PAGE_EXTENSION if !64BIT
> > 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.
> > + This feature adds 'PG_idle' and 'PG_young' flags in 'struct page'.
> > + PTE Accessed bit writers can save the state of the bit in the flags
> > + to let other PTE Accessed bit readers don't get disturbed.
>
> How about this?
>
> --- a/mm/Kconfig~mm-idle_page_tracking-make-pg_idle-reusable-fix-fix
> +++ a/mm/Kconfig
> @@ -743,9 +743,9 @@ config PAGE_IDLE_FLAG
> bool "Add PG_idle and PG_young flags"
> select PAGE_EXTENSION if !64BIT
> 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.
> + This adds PG_idle and PG_young flags to 'struct page'. PTE Accessed
> + bit writers can set the state of the bit in the flags so that PTE
> + Accessed bit readers may avoid disturbance.
>
> config IDLE_PAGE_TRACKING
> bool "Enable idle page tracking"
So good, thank you!
>
> Also, is there any way in which we can avoid presenting this option to
> the user? Because most users will have real trouble understanding what
> this thing is for. Can we simply select it when needed, as dictated by
> other, higher-level config options?
I believe this is the right way to go! I sent a patch for removing the prompt
of this option:
https://lore.kernel.org/linux-mm/[email protected]/
Thanks,
SeongJae Park