Re: [PATCH V2 19/31] powerpc/mm: Convert 4k hash insert to C

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 09/30/2015 07:57 AM, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/Makefile        |   3 +
>  arch/powerpc/mm/hash64_64k.c    | 202 +++++++++++++++++++++
>  arch/powerpc/mm/hash_low_64.S   | 380 ----------------------------------------
>  arch/powerpc/mm/hash_utils_64.c |   4 +-
>  4 files changed, 208 insertions(+), 381 deletions(-)
>  create mode 100644 arch/powerpc/mm/hash64_64k.c
> 
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 3eb73a38220d..f80ad1a76cc8 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -18,6 +18,9 @@ obj-$(CONFIG_PPC_STD_MMU_32)	+= ppc_mmu_32.o
>  obj-$(CONFIG_PPC_STD_MMU)	+= hash_low_$(CONFIG_WORD_SIZE).o \
>  				   tlb_hash$(CONFIG_WORD_SIZE).o \
>  				   mmu_context_hash$(CONFIG_WORD_SIZE).o
> +ifeq ($(CONFIG_PPC_STD_MMU_64),y)
> +obj-$(CONFIG_PPC_64K_PAGES)	+= hash64_64k.o
> +endif
>  obj-$(CONFIG_PPC_ICSWX)		+= icswx.o
>  obj-$(CONFIG_PPC_ICSWX_PID)	+= icswx_pid.o
>  obj-$(CONFIG_40x)		+= 40x_mmu.o
> diff --git a/arch/powerpc/mm/hash64_64k.c b/arch/powerpc/mm/hash64_64k.c
> new file mode 100644
> index 000000000000..b137e50a3e57
> --- /dev/null
> +++ b/arch/powerpc/mm/hash64_64k.c
> @@ -0,0 +1,202 @@
> +/*
> + * Copyright IBM Corporation, 2015
> + * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2.1 of the GNU Lesser General Public License
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + *
> + */
> +
> +#include <linux/mm.h>
> +#include <asm/machdep.h>
> +#include <asm/mmu.h>
> +
> +int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
> +		   pte_t *ptep, unsigned long trap, unsigned long flags,
> +		   int ssize, int subpg_prot)
> +{
> +	real_pte_t rpte;
> +	unsigned long *hidxp;
> +	unsigned long hpte_group;
> +	unsigned int subpg_index;
> +	unsigned long shift = 12; /* 4K */
> +	unsigned long rflags, pa, hidx;
> +	unsigned long old_pte, new_pte, subpg_pte;
> +	unsigned long vpn, hash, slot;
> +
> +	/*
> +	 * atomically mark the linux large page PTE busy and dirty
> +	 */
> +	do {
> +		pte_t pte = READ_ONCE(*ptep);
> +
> +		old_pte = pte_val(pte);
> +		/* If PTE busy, retry the access */

Small nit, need a gap between the above two lines ?

> +		if (unlikely(old_pte & _PAGE_BUSY))
> +			return 0;
> +		/* If PTE permissions don't match, take page fault */

We are already in page fault interrupt path,  will it be better
if we call it "take Linux page fault" instead as we will go back
walking the page table.