subject:"\[PATCH 19\/31\] powerpc\/mm\: Convert 4k hash insert to C"

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

2015-09-29 Thread Aneesh Kumar K.V

Benjamin Herrenschmidt  writes:

> On Mon, 2015-09-21 at 12:10 +0530, Aneesh Kumar K.V wrote:
>> Signed-off-by: Aneesh Kumar K.V 
>> ---
>>  arch/powerpc/mm/Makefile|   3 +
>>  arch/powerpc/mm/hash64_64k.c| 204 +
>>  arch/powerpc/mm/hash_low_64.S   | 380 --
>> --
>>  arch/powerpc/mm/hash_utils_64.c |   4 +-
>>  4 files changed, 210 insertions(+), 381 deletions(-)
>>  create mode 100644 arch/powerpc/mm/hash64_64k.c
>
> Did you check if there was any measurable performance difference ?
>

I looked at the performance number with and without patch. I don't see much
impact in the numbers. We do have a path lengh increase ( I measured this
using systemsim)

Path length __hash_page_4k
with patch: 196
without patch: 142

Path length __hash_page_64k
with patch: 219
without patch: 154

But even if we have a path lengh increase of around 50 instructions. We don't 
see
the impact when running workload. I tried the kernelbuild test. 

With THP enabled (which is default) we see an improvement. I haven't fully 
looked at
the reason. This could be due to reduced contention of ptl lock. 
__hash_thp_page is
already a C code.

make -j64 vmlinux modules 
With fix:
-
real1m35.509s
user56m8.565s
sys 4m34.973s

real1m32.174s
user57m2.336s
sys 4m39.142s

Without fix:
---
real1m37.703s
user58m50.783s
sys 7m52.440s

real1m37.890s
user57m55.445s
sys 7m50.501s

THP disabled:

make -j64 vmlinux modules 
With fix:
-
real1m37.197s
user58m28.672s
sys 7m58.188s

real1m44.638s
user58m37.551s
sys 7m53.960s

Without fix:

real1m41.224s
user58m46.944s
sys 7m49.714s

real1m42.585s
user59m14.019s
sys 7m52.714s

-aneesh

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-09-21 Thread Aneesh Kumar K.V

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/mm/Makefile|   3 +
 arch/powerpc/mm/hash64_64k.c| 204 +
 arch/powerpc/mm/hash_low_64.S   | 380 
 arch/powerpc/mm/hash_utils_64.c |   4 +-
 4 files changed, 210 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 ..cbbef40c549a
--- /dev/null
+++ b/arch/powerpc/mm/hash64_64k.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright IBM Corporation, 2015
+ * Author Aneesh Kumar K.V 
+ *
+ * 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 
+#include 
+#include 
+
+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 */
+   if (unlikely(old_pte & _PAGE_BUSY))
+   return 0;
+   /* If PTE permissions don't match, take page fault */
+   if (unlikely(access & ~old_pte))
+   return 1;
+   /*
+* Try to lock the PTE, add ACCESSED and DIRTY if it was
+* a write access. Since this is 4K insert of 64K page size
+* also add _PAGE_COMBO
+*/
+   new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED | _PAGE_COMBO;
+   if (access & _PAGE_RW)
+   new_pte |= _PAGE_DIRTY;
+   } while (old_pte != __cmpxchg_u64((unsigned long *)ptep,
+ old_pte, new_pte));
+   /*
+* Handle the subpage protection bits
+*/
+   subpg_pte = new_pte & ~subpg_prot;
+   /*
+* PP bits. _PAGE_USER is already PP bit 0x2, so we only
+* need to add in 0x1 if it's a read-only user page
+*/
+   rflags = subpg_pte & _PAGE_USER;
+   if ((subpg_pte & _PAGE_USER) && !((subpg_pte & _PAGE_RW) &&
+   (subpg_pte & _PAGE_DIRTY)))
+   rflags |= 0x1;
+   /*
+* _PAGE_EXEC -> HW_NO_EXEC since it's inverted
+*/
+   rflags |= ((subpg_pte & _PAGE_EXEC) ? 0 : HPTE_R_N);
+   /*
+* Always add C and Memory coherence bit
+*/
+   rflags |= HPTE_R_C | HPTE_R_M;
+   /*
+* Add in WIMG bits
+*/
+   rflags |= (subpg_pte & (_PAGE_WRITETHRU | _PAGE_NO_CACHE |
+   _PAGE_COHERENT | _PAGE_GUARDED));
+   /*
+* FIXME!! check this
+*/
+   if (!cpu_has_feature(CPU_FTR_NOEXECUTE) &&
+   !cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
+
+   /*
+* No CPU has hugepages but lacks no execute, so we
+* don't need to worry about that case
+*/
+   rflags = hash_page_do_lazy_icache(rflags, __pte(old_pte), trap);
+   }
+
+   subpg_index = (ea & (PAGE_SIZE - 1)) >> shift;
+   vpn  = hpt_vpn(ea, vsid, ssize);
+   rpte = __real_pte(old_pte, ptep);
+   /*
+*None of the sub 4k page is hashed
+*/
+   if (!(old_pte & _PAGE_HASHPTE))
+   goto htab_insert_hpte;
+   /*
+* Check if the pte was already inserted into the hash

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

2015-09-21 Thread Benjamin Herrenschmidt

On Mon, 2015-09-21 at 12:10 +0530, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V 
> ---
>  arch/powerpc/mm/Makefile|   3 +
>  arch/powerpc/mm/hash64_64k.c| 204 +
>  arch/powerpc/mm/hash_low_64.S   | 380 --
> --
>  arch/powerpc/mm/hash_utils_64.c |   4 +-
>  4 files changed, 210 insertions(+), 381 deletions(-)
>  create mode 100644 arch/powerpc/mm/hash64_64k.c

Did you check if there was any measurable performance difference ?

Cheers,
Ben.

> 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 ..cbbef40c549a
> --- /dev/null
> +++ b/arch/powerpc/mm/hash64_64k.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright IBM Corporation, 2015
> + * Author Aneesh Kumar K.V 
> + *
> + * 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 
> +#include 
> +#include 
> +
> +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 */
> + if (unlikely(old_pte & _PAGE_BUSY))
> + return 0;
> + /* If PTE permissions don't match, take page fault
> */
> + if (unlikely(access & ~old_pte))
> + return 1;
> + /*
> +  * Try to lock the PTE, add ACCESSED and DIRTY if it
> was
> +  * a write access. Since this is 4K insert of 64K
> page size
> +  * also add _PAGE_COMBO
> +  */
> + new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED |
> _PAGE_COMBO;
> + if (access & _PAGE_RW)
> + new_pte |= _PAGE_DIRTY;
> + } while (old_pte != __cmpxchg_u64((unsigned long *)ptep,
> +   old_pte, new_pte));
> + /*
> +  * Handle the subpage protection bits
> +  */
> + subpg_pte = new_pte & ~subpg_prot;
> + /*
> +  * PP bits. _PAGE_USER is already PP bit 0x2, so we only
> +  * need to add in 0x1 if it's a read-only user page
> +  */
> + rflags = subpg_pte & _PAGE_USER;
> + if ((subpg_pte & _PAGE_USER) && !((subpg_pte & _PAGE_RW) &&
> + (subpg_pte & _PAGE_DIRTY)))
> + rflags |= 0x1;
> + /*
> +  * _PAGE_EXEC -> HW_NO_EXEC since it's inverted
> +  */
> + rflags |= ((subpg_pte & _PAGE_EXEC) ? 0 : HPTE_R_N);
> + /*
> +  * Always add C and Memory coherence bit
> +  */
> + rflags |= HPTE_R_C | HPTE_R_M;
> + /*
> +  * Add in WIMG bits
> +  */
> + rflags |= (subpg_pte & (_PAGE_WRITETHRU | _PAGE_NO_CACHE |
> + _PAGE_COHERENT | _PAGE_GUARDED));
> + /*
> +  * FIXME!! check this
> +  */
> + if (!cpu_has_feature(CPU_FTR_NOEXECUTE) &&
> + !cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
> +
> + /*
> +  * No CPU has hugepages but lacks no execute, so we
> +  * don't need to worry about that case
> +  */
> + rflags = hash_page_do_lazy_icache(rflags,
> __pte(old_pte), trap);
> + }
> +
> + subpg_index = (ea & (PAGE_SIZE - 1)) >> shift;
> + vpn  = hpt_vpn(ea, vsid,

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

2015-09-21 Thread Aneesh Kumar K.V

Benjamin Herrenschmidt  writes:

> On Mon, 2015-09-21 at 12:10 +0530, Aneesh Kumar K.V wrote:
>> Signed-off-by: Aneesh Kumar K.V 
>> ---
>>  arch/powerpc/mm/Makefile|   3 +
>>  arch/powerpc/mm/hash64_64k.c| 204 +
>>  arch/powerpc/mm/hash_low_64.S   | 380 --
>> --
>>  arch/powerpc/mm/hash_utils_64.c |   4 +-
>>  4 files changed, 210 insertions(+), 381 deletions(-)
>>  create mode 100644 arch/powerpc/mm/hash64_64k.c
>
> Did you check if there was any measurable performance difference ?
>

Not with this patch series. I did look at the .s file generated between
__hash_page_4k and hash64_4k.c and found them similar. 

I will also try some micro benchmarks w.r.t hash insert/update and will
update later with numbers.

-aneesh

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information