There is 8 byte page_ext->flags field allocated per page whenever
CONFIG_PAGE_EXTENSION is enabled. However, not every user of page_ext
uses flags. Therefore, check whether flags is needed at least by one
user and if so allocate space for it.
For example when page_table_check is enabled, on a machine with 128G
of memory before the fix:
[ 2.244288] allocated 536870912 bytes of page_ext
after the fix:
[ 2.160154] allocated 268435456 bytes of page_ext
Signed-off-by: Pasha Tatashin <[email protected]>
---
include/linux/page_ext.h | 7 +++++++
mm/page_ext.c | 12 +++++++++++-
mm/page_owner.c | 1 +
mm/page_table_check.c | 1 +
4 files changed, 20 insertions(+), 1 deletion(-)
Changelog:
v2: Fixed field name in page_owner.c that caused build error.
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 22be4582faae..29909e135052 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -12,10 +12,14 @@ struct page_ext_operations {
size_t size;
bool (*need)(void);
void (*init)(void);
+ bool using_shared_ext_flags;
};
#ifdef CONFIG_PAGE_EXTENSION
+/*
+ * The page_ext_flags users must set using_shared_ext_flags to true.
+ */
enum page_ext_flags {
PAGE_EXT_OWNER,
PAGE_EXT_OWNER_ALLOCATED,
@@ -31,6 +35,9 @@ enum page_ext_flags {
* page_ext helps us add more information about the page.
* All page_ext are allocated at boot or memory hotplug event,
* then the page_ext for pfn always exists.
+ * Note: that if none of the users of page_ext is setting
+ * using_shared_ext_flags to true, the field cannot be used,
+ * as it will be overlapped with another data.
*/
struct page_ext {
unsigned long flags;
diff --git a/mm/page_ext.c b/mm/page_ext.c
index 4ee522fd381c..7443a797b96b 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -71,6 +71,7 @@ static bool need_page_idle(void)
}
static struct page_ext_operations page_idle_ops __initdata = {
.need = need_page_idle,
+ .using_shared_ext_flags = true,
};
#endif
@@ -86,7 +87,7 @@ static struct page_ext_operations *page_ext_ops[] __initdata = {
#endif
};
-unsigned long page_ext_size = sizeof(struct page_ext);
+unsigned long page_ext_size;
static unsigned long total_usage;
static struct page_ext *lookup_page_ext(const struct page *page);
@@ -105,6 +106,15 @@ static bool __init invoke_need_callbacks(void)
int entries = ARRAY_SIZE(page_ext_ops);
bool need = false;
+ for (i = 0; i < entries; i++) {
+ if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
+ if (page_ext_ops[i]->using_shared_ext_flags) {
+ page_ext_size = sizeof(struct page_ext);
+ break;
+ }
+ }
+ }
+
for (i = 0; i < entries; i++) {
if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
page_ext_ops[i]->offset = page_ext_size;
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 2d27f532df4c..1a62ab3fc00e 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -99,6 +99,7 @@ struct page_ext_operations page_owner_ops = {
.size = sizeof(struct page_owner),
.need = need_page_owner,
.init = init_page_owner,
+ .using_shared_ext_flags = true,
};
static inline struct page_owner *get_page_owner(struct page_ext *page_ext)
diff --git a/mm/page_table_check.c b/mm/page_table_check.c
index 93e633c1d587..74cfe89d5f53 100644
--- a/mm/page_table_check.c
+++ b/mm/page_table_check.c
@@ -45,6 +45,7 @@ struct page_ext_operations page_table_check_ops = {
.size = sizeof(struct page_table_check),
.need = need_page_table_check,
.init = init_page_table_check,
+ .using_shared_ext_flags = false,
};
static struct page_table_check *get_page_table_check(struct page_ext *page_ext)
--
2.39.0.314.g84b9a713c41-goog
On Thu, 12 Jan 2023 15:33:48 +0000 Pasha Tatashin <[email protected]> wrote:
> There is 8 byte page_ext->flags field allocated per page whenever
> CONFIG_PAGE_EXTENSION is enabled. However, not every user of page_ext
> uses flags. Therefore, check whether flags is needed at least by one
> user and if so allocate space for it.
>
> For example when page_table_check is enabled, on a machine with 128G
> of memory before the fix:
>
> [ 2.244288] allocated 536870912 bytes of page_ext
> after the fix:
> [ 2.160154] allocated 268435456 bytes of page_ext
>
Looks nice.
Does it work correctly with early_page_ext=1?
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -12,10 +12,14 @@ struct page_ext_operations {
> size_t size;
> bool (*need)(void);
> void (*init)(void);
> + bool using_shared_ext_flags;
> };
Seems overly complicated. Can we change the three early-init
functions such as setup_early_page_ext() to simply set some global bool
flag?
> #endif
> };
>
> -unsigned long page_ext_size = sizeof(struct page_ext);
> +unsigned long page_ext_size;
>
> static unsigned long total_usage;
> static struct page_ext *lookup_page_ext(const struct page *page);
> @@ -105,6 +106,15 @@ static bool __init invoke_need_callbacks(void)
> int entries = ARRAY_SIZE(page_ext_ops);
> bool need = false;
>
> + for (i = 0; i < entries; i++) {
> + if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
I don't think this (or the below) need to check for ->need==NULL?
> + if (page_ext_ops[i]->using_shared_ext_flags) {
> + page_ext_size = sizeof(struct page_ext);
> + break;
> + }
> + }
> + }
> +
> for (i = 0; i < entries; i++) {
> if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
> page_ext_ops[i]->offset = page_ext_size;
On Thu, Jan 12, 2023 at 4:29 PM Andrew Morton <[email protected]> wrote:
>
> On Thu, 12 Jan 2023 15:33:48 +0000 Pasha Tatashin <[email protected]> wrote:
>
> > There is 8 byte page_ext->flags field allocated per page whenever
> > CONFIG_PAGE_EXTENSION is enabled. However, not every user of page_ext
> > uses flags. Therefore, check whether flags is needed at least by one
> > user and if so allocate space for it.
> >
> > For example when page_table_check is enabled, on a machine with 128G
> > of memory before the fix:
> >
> > [ 2.244288] allocated 536870912 bytes of page_ext
> > after the fix:
> > [ 2.160154] allocated 268435456 bytes of page_ext
> >
>
> Looks nice.
Hi Andrew,
Thanks for looking at this patch.
>
> Does it work correctly with early_page_ext=1?
Yes, early_page_ext=1 moves page_ext_init() earlier, and disables
deferred page initialization, but that is not an issue for the changes
in this patch.
>
> > --- a/include/linux/page_ext.h
> > +++ b/include/linux/page_ext.h
> > @@ -12,10 +12,14 @@ struct page_ext_operations {
> > size_t size;
> > bool (*need)(void);
> > void (*init)(void);
> > + bool using_shared_ext_flags;
> > };
>
> Seems overly complicated. Can we change the three early-init
> functions such as setup_early_page_ext() to simply set some global bool
> flag?
We can't because , in the following loop:
for (i = 0; i < entries; i++) {
if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
page_ext_ops[i]->offset = page_ext_size;
page_ext_size must already either include or not the space for the
shared flag field.
>
> > #endif
> > };
> >
> > -unsigned long page_ext_size = sizeof(struct page_ext);
> > +unsigned long page_ext_size;
> >
> > static unsigned long total_usage;
> > static struct page_ext *lookup_page_ext(const struct page *page);
> > @@ -105,6 +106,15 @@ static bool __init invoke_need_callbacks(void)
> > int entries = ARRAY_SIZE(page_ext_ops);
> > bool need = false;
> >
> > + for (i = 0; i < entries; i++) {
> > + if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
>
> I don't think this (or the below) need to check for ->need==NULL?
Good point, I will remove the check, and also update the comment that
the "need" function must be defined for every item in the page_ext_ops
array.
Thanks,
Pasha