Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Thu, 12 Jan 2023 13:29:57 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH v2] mm/page_ext: Do not allocate space for page_ext->flags if not needed |
| |
On Thu, 12 Jan 2023 15:33:48 +0000 Pasha Tatashin <pasha.tatashin@soleen.com> 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;
| ![\](/images/icornerr.gif) |