linux-kernel - Re: [PATCH v2] mm/page_ext: Do not allocate space for page

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230112132957.6cb97c2afad17cdeb06fcd10@linux-foundation.org>
Date:   Thu, 12 Jan 2023 13:29:57 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Pasha Tatashin <pasha.tatashin@...een.com>
Cc:     mhocko@...e.com, vbabka@...e.cz, david@...hat.com,
        quic_charante@...cinc.com, lizhe.67@...edance.com,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
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@...een.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;