linux-kernel - Re: [PATCH v2 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] [day] [month] [year] [list]

Message-ID: <ZS/m1KRwTLkcJY8y@casper.infradead.org>
Date:   Wed, 18 Oct 2023 15:08:20 +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 v2 1/1] mm: report per-page metadata information

On Tue, Oct 17, 2023 at 05:55:48PM -0700, Sourav Panda wrote:
> +		mod_node_early_perpage_metadata(pgdat->node_id,
> +						PAGE_ALIGN(size) >> PAGE_SHIFT);

What a curious way of writing DIV_ROUND_UP(size, PAGE_SIZE)
(throughout).  A quick grep shows about 230 DIV_ROUND_UP and 110 of what
you wrote, so it's not something you invented, but DIV_ROUND_UP is
clearer.

> @@ -303,18 +308,25 @@ static int __meminit init_section_page_ext(unsigned long pfn, int nid)
>  
>  static void free_page_ext(void *addr)
>  {
> +	size_t table_size;
> +	struct page *page;
> +
> +	table_size = page_ext_size * PAGES_PER_SECTION;
> +
>  	if (is_vmalloc_addr(addr)) {
> +		page = vmalloc_to_page(addr);
>  		vfree(addr);
>  	} else {
> -		struct page *page = virt_to_page(addr);
> -		size_t table_size;
> -
> -		table_size = page_ext_size * PAGES_PER_SECTION;
> +		page = virt_to_page(addr);
>  
>  		BUG_ON(PageReserved(page));
>  		kmemleak_free(addr);
>  		free_pages_exact(addr, table_size);
>  	}
> +
> +	__mod_node_page_state(page_pgdat(page), NR_PAGE_METADATA,
> +			      -1L * (PAGE_ALIGN(table_size) >> PAGE_SHIFT));

This troubles me.  We're freeing the memory and then dereferencing
the page that was freed.  I know that struct pages don't go away when
they're freed, and they don't change which node they're allocated to,
but it feels wrong.  I'd be happier if the page_pgdat() were extracted
prior to freeing the memory.