linux-kernel - Re: [PATCH v2 6/9] x86/clear_huge

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: <20230908123819.GB19320@noisy.programming.kicks-ass.net>
Date:   Fri, 8 Sep 2023 14:38:19 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Ankur Arora <ankur.a.arora@...cle.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
        akpm@...ux-foundation.org, luto@...nel.org, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com, mingo@...hat.com,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        willy@...radead.org, mgorman@...e.de, rostedt@...dmis.org,
        tglx@...utronix.de, jon.grimm@....com, bharata@....com,
        raghavendra.kt@....com, boris.ostrovsky@...cle.com,
        konrad.wilk@...cle.com
Subject: Re: [PATCH v2 6/9] x86/clear_huge_page: multi-page clearing

On Wed, Aug 30, 2023 at 11:49:55AM -0700, Ankur Arora wrote:

> +#ifndef CONFIG_HIGHMEM

I'm thinking this wants to be: #ifdef CONFIG_X86_64. All the previous
stuff was 64bit specific.

> +static void clear_contig_region(struct page *page, unsigned int npages)
> +{
> +	clear_pages(page_address(page), npages);
> +}

I'm not sure about the naming of this helper -- but whatever.

> +
> +/*
> + * clear_huge_page(): multi-page clearing variant of clear_huge_page().
> + *
> + * Taking inspiration from the common code variant, we split the zeroing in
> + * three parts: left of the fault, right of the fault, and up to 5 pages
> + * in the immediate neighbourhood of the target page.
> + *
> + * Cleared in that order to keep cache lines of the target region hot.
> + *
> + * For gigantic pages, there is no expectation of cache locality so we do a
> + * straight zeroing.
> + */
> +void clear_huge_page(struct page *page,
> +		     unsigned long addr_hint, unsigned int pages_per_huge_page)
> +{
> +	unsigned long addr = addr_hint &
> +		~(((unsigned long)pages_per_huge_page << PAGE_SHIFT) - 1);
> +	const long pgidx = (addr_hint - addr) / PAGE_SIZE;
> +	const int first_pg = 0, last_pg = pages_per_huge_page - 1;
> +	const int width = 2; /* pages cleared last on either side */
> +	int sidx[3], eidx[3];
> +	int i, n;
> +
> +	if (pages_per_huge_page > MAX_ORDER_NR_PAGES)
> +		return clear_contig_region(page, pages_per_huge_page);
> +
> +	/*
> +	 * Neighbourhood of the fault. Cleared at the end to ensure
> +	 * it sticks around in the cache.
> +	 */
> +	n = 2;
> +	sidx[n] = (pgidx - width) < first_pg ? first_pg : (pgidx - width);
> +	eidx[n] = (pgidx + width) > last_pg  ? last_pg  : (pgidx + width);
> +
> +	sidx[0] = first_pg;	/* Region to the left of the fault */
> +	eidx[0] = sidx[n] - 1;
> +
> +	sidx[1] = eidx[n] + 1;	/* Region to the right of the fault */
> +	eidx[1] = last_pg;
> +
> +	for (i = 0; i <= 2; i++) {
> +		if (eidx[i] >= sidx[i])
> +			clear_contig_region(page + sidx[i],
> +					    eidx[i] - sidx[i] + 1);

Since the if has a multi-line statement it needs { } per coding style.

> +	}
> +}
> +#endif /* CONFIG_HIGHMEM */
>  #endif /* CONFIG_HUGETLB_PAGE */
>  
>  #ifdef CONFIG_X86_64
> -- 
> 2.31.1
>