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

[Dave Hansen]

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@xxxxxxxxx>