linux-kernel - Re: [patch V3 09/11] x86/mm/cpa: Optimize same protection check

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: <9bf16a03-d54f-d424-46fb-d16a80ec5cc5@intel.com>
Date:   Fri, 21 Sep 2018 13:12:58 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Cc:     x86@...nel.org, Peter Zijlstra <peterz@...radead.org>,
        Bin Yang <bin.yang@...el.com>,
        Mark Gross <mark.gross@...el.com>
Subject: Re: [patch V3 09/11] x86/mm/cpa: Optimize same protection check

On 09/17/2018 07:29 AM, Thomas Gleixner wrote:
> When the existing mapping is correct and the new requested page protections
> are the same as the existing ones, then further checks can be omitted and the
> large page can be preserved. The slow path 4k wise check will not come up with
> a different result.
> 
> Before:
> 
>  1G pages checked:                    2
>  1G pages sameprot:                   0
>  1G pages preserved:                  0
>  2M pages checked:                  540
>  2M pages sameprot:                 466
>  2M pages preserved:                 47
>  4K pages checked:               800709
>  4K pages set-checked:             7668
> 
> After:
> 
>  1G pages checked:                    2
>  1G pages sameprot:                   0
>  1G pages preserved:                  0
>  2M pages checked:                  538
>  2M pages sameprot:                 466
>  2M pages preserved:                 47
>  4K pages checked:               560642
>  4K pages set-checked:             7668

With this new path added, I would have expected "2M pages sameprot" or
"1G pages sameprot" to go up.  The "checked" pages go down, which makes
sense, but I don't see either of the "sameprot"s going up.

I did a quick look back at the stats patch and didn't see any obvious
buglets that can account for it.  Both that and this code look sane, but
the stats just don't seem to square for some reason.

>  	/*
> +	 * Optimization: If the requested pgprot is the same as the current
> +	 * pgprot, then the large page can be preserved and no updates are
> +	 * required independent of alignment and length of the requested
> +	 * range. The above already established that the current pgprot is
> +	 * correct, which in consequence makes the requested pgprot correct
> +	 * as well if it is the same. The static protection scan below will
> +	 * not come to a different conclusion.
> +	 */
> +	if (pgprot_val(req_prot) == pgprot_val(old_prot)) {
> +		cpa_inc_lp_sameprot(level);
> +		return 0;
> +	}

Patch *looks* fine though.  I'm going to assume that the pasted stats
were from the wrong run or something.

Reviewed-by: Dave Hansen <dave.hansen@...el.com>