linux-kernel - Re: [PATCH v1 1/1] mm: report per-page metadata information

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: <ZQH3tHbz2ghsyqHG@casper.infradead.org>
Date:   Wed, 13 Sep 2023 18:56:04 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Sourav Panda <souravpanda@...gle.com>
Cc:     corbet@....net, gregkh@...uxfoundation.org, rafael@...nel.org,
        akpm@...ux-foundation.org, mike.kravetz@...cle.com,
        muchun.song@...ux.dev, rppt@...nel.org, david@...hat.com,
        rdunlap@...radead.org, chenlinxuan@...ontech.com,
        yang.yang29@....com.cn, tomas.mudrunka@...il.com,
        bhelgaas@...gle.com, ivan@...udflare.com,
        pasha.tatashin@...een.com, yosryahmed@...gle.com,
        hannes@...xchg.org, shakeelb@...gle.com,
        kirill.shutemov@...ux.intel.com, wangkefeng.wang@...wei.com,
        adobriyan@...il.com, vbabka@...e.cz, Liam.Howlett@...cle.com,
        surenb@...gle.com, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v1 1/1] mm: report per-page metadata information

On Wed, Sep 13, 2023 at 10:30:00AM -0700, Sourav Panda wrote:
> @@ -387,8 +390,12 @@ static int alloc_vmemmap_page_list(unsigned long start, unsigned long end,
>  
>  	while (nr_pages--) {
>  		page = alloc_pages_node(nid, gfp_mask, 0);
> -		if (!page)
> +		if (!page) {
>  			goto out;
> +		} else {
> +			__mod_node_page_state(NODE_DATA(page_to_nid(page)),
> +					      NR_PAGE_METADATA, 1);
> +		}
>  		list_add_tail(&page->lru, list);

What a strange way of writing this.  Why not simply:

		if (!page)
			goto out;
+		__mod_node_page_state(NODE_DATA(page_to_nid(page)),
+				NR_PAGE_METADATA, 1);
		list_add_tail(&page->lru, list);

> @@ -314,6 +319,10 @@ static void free_page_ext(void *addr)
>  		BUG_ON(PageReserved(page));
>  		kmemleak_free(addr);
>  		free_pages_exact(addr, table_size);
> +
> +		__mod_node_page_state(NODE_DATA(page_to_nid(page)), NR_PAGE_METADATA,
> +				      (long)-1 * (PAGE_ALIGN(table_size) >> PAGE_SHIFT));

Why not spell that as "-1L"?

And while I'm asking questions, why NODE_DATA(page_to_nid(page)) instead
of page_pgdat(page)?

> @@ -2274,4 +2275,24 @@ static int __init extfrag_debug_init(void)
>  }
>  
>  module_init(extfrag_debug_init);
> +
> +// Page metadata size (struct page and page_ext) in pages

Don't use // comments.

> +void __init writeout_early_perpage_metadata(void)

"writeout" is something swap does.  I'm sure this has a better name,
though I can't think what it might be.

> +{
> +	int nid;
> +	struct pglist_data *pgdat;
> +
> +	for_each_online_pgdat(pgdat) {
> +		nid = pgdat->node_id;
> +		__mod_node_page_state(NODE_DATA(nid), NR_PAGE_METADATA,
> +				      early_perpage_metadata[nid]);
> +	}
> +}
>  #endif
> -- 
> 2.42.0.283.g2d96d420d3-goog
>