Re: [PATCH] x86/mm: Avoid paravirtualization calls in native_set_p4d()

[Thomas Gleixner]

On Wed, 28 Feb 2018, Kirill A. Shutemov wrote:

> p4d_val() and pgd_val() are paravirtualized and we should avoid using
> them in native_set_p4d().

I'm really unhappy with your changelogs.

... and we should avoid ....

Should is not strong enough as it leaves the option to not do it.

Aside of that you fail (again) to explain the WHY. Something like this:

native_set_p4d() uses p4d_val() and pgd_val() to do ".....". Both functions
are paravirtualized which is wrong because it brings paravirtualization
into the native implementation resulting in "$problem".

> Let's replace them with native_p4d_val() and native_pgd_val().

Let's? Just say:

Replace them with native_p4d_val() and native_pgd_val().

Documentation/process/... explains it really well change logs have to be
written in imperative mood.

Thanks,

	tglx

> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Reported-by: Fengguang Wu <fengguang.wu@xxxxxxxxx>
> Fixes: 91f606a8fa68 ("x86/mm: Replace compile-time checks for 5-level paging with runtime-time checks")
> ---
>  arch/x86/include/asm/pgtable_64.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 81dda8d1d0bd..163e01a0631d 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -224,9 +224,9 @@ static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d)
>  		return;
>  	}
>  
> -	pgd = native_make_pgd(p4d_val(p4d));
> +	pgd = native_make_pgd(native_p4d_val(p4d));
>  	pgd = pti_set_user_pgd((pgd_t *)p4dp, pgd);
> -	*p4dp = native_make_p4d(pgd_val(pgd));
> +	*p4dp = native_make_p4d(native_pgd_val(pgd));
>  }
>  
>  static inline void native_p4d_clear(p4d_t *p4d)
> -- 
> 2.16.1
> 
>