[PATCH 0/2] mm/thp: rework the pmd protect changing flow

[PATCH 0/2] mm/thp: rework the pmd protect changing flow

[Xuefeng Wang]

On KunPeng920 board. When changing permission of a large range region,
pmdp_invalidate() takes about 65% in profile (with hugepages) in JIT tool.
Kernel will flush tlb twice: first flush happens in pmdp_invalidate, second
flush happens at the end of change_protect_range(). The first pmdp_invalidate
is not necessary if the hardware support atomic pmdp changing. The atomic
changing pimd to zero can prevent the hardware from update asynchronous.
So reconstruct it and remove the first pmdp_invalidate. And the second tlb
flush can make sure the new tlb entry valid.

This patch series add a pmdp_modify_prot transaction abstraction firstly.
Then add pmdp_modify_prot_start() in arm64, which uses pmdp_huge_get_and_clear()
to atomically fetch the pmd and zero the entry.

After rework, the mprotect can get 3~13 times performace gain in range
64M to 512M on KunPeng920:

4K granule/THP on
memory size(M)	64	128	256	320	448	512
pre-patch		0.77	1.40	2.64	3.23	4.49	5.10
post-patch		0.20	0.23	0.28	0.31	0.37	0.39

Xuefeng Wang (2):
  mm: add helpers pmdp_modify_prot_start/commit
  arm64: mm: rework the pmd protect changing flow

 arch/arm64/include/asm/pgtable.h | 14 +++++++++++++
 include/asm-generic/pgtable.h    | 35 ++++++++++++++++++++++++++++++++
 mm/huge_memory.c                 | 19 ++++++++---------
 3 files changed, 57 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH 1/2] mm: add helpers pmdp_modify_prot_start/commit

[Xuefeng Wang]

Introduce helpers pmdp_modify_prot_start/commit to abstract pmdp_modify_prot
transaction. Helpers pmdp_modify_prot_start/commit are functionally unchanged.

Signed-off-by: Xuefeng Wang <wxf.wang@hisilicon.com>
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
 include/asm-generic/pgtable.h | 35 +++++++++++++++++++++++++++++++++++
 mm/huge_memory.c              | 19 ++++++++-----------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 798ea36a0549..e81bd58a9170 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -673,6 +673,41 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
 	__ptep_modify_prot_commit(vma, addr, ptep, pte);
 }
 #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */
+
+static inline pmd_t __pmdp_modify_prot_start(struct vm_area_struct *vma,
+						unsigned long addr,
+						pmd_t *pmdp)
+{
+	return pmdp_invalidate(vma, addr, pmdp);
+}
+
+static inline void __pmdp_modify_prot_commit(struct vm_area_struct *vma,
+						unsigned long addr,
+						pmd_t *pmdp, pmd_t pmd)
+{
+	set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
+}
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#ifndef __HAVE_ARCH_PMDP_MODIFY_PROT_TRANSACTION
+static inline pmd_t pmdp_modify_prot_start(struct vm_area_struct *vma,
+						unsigned long addr,
+						pmd_t *pmdp)
+{
+	__pmdp_modify_prot_start(vma, addr, pmdp);
+}
+#endif /* __HAVE_ARCH_PMDP_MODIFY_PROT_TRANSACTION */
+
+/*
+ * Commit an update to a pmd.
+ */
+static inline void pmdp_modify_prot_commit(struct vm_area_struct *vma,
+						unsigned long addr,
+						pmd_t *pmdp, pmd_t old_pmd, pmd_t pmd)
+{
+	__pmdp_modify_prot_commit(vma, addr, pmdp, pmd);
+}
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 #endif /* CONFIG_MMU */
 
 /*
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index a88093213674..53515a3c91dd 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1933,9 +1933,8 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
 int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 		unsigned long addr, pgprot_t newprot, int prot_numa)
 {
-	struct mm_struct *mm = vma->vm_mm;
 	spinlock_t *ptl;
-	pmd_t entry;
+	pmd_t pmdnt, oldpmd;
 	bool preserve_write;
 	int ret;
 
@@ -1961,7 +1960,7 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 			newpmd = swp_entry_to_pmd(entry);
 			if (pmd_swp_soft_dirty(*pmd))
 				newpmd = pmd_swp_mksoft_dirty(newpmd);
-			set_pmd_at(mm, addr, pmd, newpmd);
+			set_pmd_at(vma->vm_mm, addr, pmd, newpmd);
 		}
 		goto unlock;
 	}
@@ -1995,18 +1994,16 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 	 *
 	 * The race makes MADV_DONTNEED miss the huge pmd and don't clear it
 	 * which may break userspace.
-	 *
-	 * pmdp_invalidate() is required to make sure we don't miss
-	 * dirty/young flags set by hardware.
 	 */
-	entry = pmdp_invalidate(vma, addr, pmd);
 
-	entry = pmd_modify(entry, newprot);
+	oldpmd = pmdp_modify_prot_start(vma, addr, pmd);
+	pmdnt = pmd_modify(oldpmd, newprot);
 	if (preserve_write)
-		entry = pmd_mk_savedwrite(entry);
+		pmdnt = pmd_mk_savedwrite(pmdnt);
+	pmdp_modify_prot_commit(vma, addr, pmd, oldpmd, pmdnt);
+
 	ret = HPAGE_PMD_NR;
-	set_pmd_at(mm, addr, pmd, entry);
-	BUG_ON(vma_is_anonymous(vma) && !preserve_write && pmd_write(entry));
+	BUG_ON(vma_is_anonymous(vma) && !preserve_write && pmd_write(pmdnt));
 unlock:
 	spin_unlock(ptl);
 	return ret;
-- 
2.17.1

[PATCH 2/2] arm64: mm: rework the pmd protect changing flow

[Xuefeng Wang]

On KunPeng920 board. When changing permission of a large range region,
pmdp_invalidate() takes about 65% in profile (with hugepages) in JIT tool.
Kernel will flush tlb twice: first flush happens in pmdp_invalidate, second
flush happens at the end of change_protect_range(). The first pmdp_invalidate
is not necessary if the hardware support atomic pmdp changing. The atomic
changing pimd to zero can prevent the hardware from update asynchronous.
So reconstruct it and remove the first pmdp_invalidate. And the second tlb
flush can make sure the new tlb entry valid.

Add pmdp_modify_prot_start() in arm64, which uses pmdp_huge_get_and_clear()
to fetch the pmd and zero entry, preventing racing of any hardware updates.

After rework, the mprotect can get 3~13 times performace gain in range
64M to 512M.

4K granule/THP on
memory size(M)	64	128	256	320	448	512
pre-patch		0.77	1.40	2.64	3.23	4.49	5.10
post-patch		0.20	0.23	0.28	0.31	0.37	0.39

Signed-off-by: Xuefeng Wang <wxf.wang@hisilicon.com>
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
 arch/arm64/include/asm/pgtable.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index cd5de0e40bfa..bccdaa5bd5f2 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -769,6 +769,20 @@ static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm,
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#define __HAVE_ARCH_PMDP_MODIFY_PROT_TRANSACTION
+static inline pmd_t pmdp_modify_prot_start(struct vm_area_struct *vma,
+						unsigned long addr,
+						pmd_t *pmdp)
+{
+	/*
+	 * Atomic change pmd to zero, prevent the hardware from update
+	 * aynchronously update it.
+	 */
+	return pmdp_huge_get_and_clear(vma->vm_mm, addr, pmdp);
+}
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
 /*
  * ptep_set_wrprotect - mark read-only while trasferring potential hardware
  * dirty status (PTE_DBM && !PTE_RDONLY) to the software PTE_DIRTY bit.
-- 
2.17.1

Re: [PATCH 1/2] mm: add helpers pmdp_modify_prot_start/commit

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

Hi Xuefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mmotm/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Xuefeng-Wang/mm-thp-rework-the-pmd-protect-changing-flow/20200117-115821
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/nds32/include/asm/pgtable.h:394,
                    from include/linux/mm.h:99,
                    from include/linux/ring_buffer.h:5,
                    from include/linux/trace_events.h:6,
                    from include/trace/syscall.h:7,
                    from include/linux/syscalls.h:85,
                    from init/main.c:21:
   include/asm-generic/pgtable.h: In function '__pmdp_modify_prot_commit':
>> include/asm-generic/pgtable.h:677:2: error: implicit declaration of function 'set_pmd_at'; did you mean 'set_pte_at'? [-Werror=implicit-function-declaration]
     677 |  set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
         |  ^~~~~~~~~~
         |  set_pte_at
   cc1: some warnings being treated as errors
--
   In file included from arch/nds32/include/asm/pgtable.h:394,
                    from include/linux/mm.h:99,
                    from arch/nds32/include/asm/uaccess.h:14,
                    from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/ptrace.h:7,
                    from arch/nds32/kernel/signal.c:6:
   include/asm-generic/pgtable.h: In function '__pmdp_modify_prot_commit':
>> include/asm-generic/pgtable.h:677:2: error: implicit declaration of function 'set_pmd_at'; did you mean 'set_pte_at'? [-Werror=implicit-function-declaration]
     677 |  set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
         |  ^~~~~~~~~~
         |  set_pte_at
   arch/nds32/kernel/signal.c: In function 'do_signal':
   arch/nds32/kernel/signal.c:362:20: warning: this statement may fall through [-Wimplicit-fallthrough=]
     362 |    regs->uregs[15] = __NR_restart_syscall;
   arch/nds32/kernel/signal.c:363:3: note: here
     363 |   case -ERESTARTNOHAND:
         |   ^~~~
   arch/nds32/kernel/signal.c: In function 'handle_signal':
   arch/nds32/kernel/signal.c:315:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
     315 |    if (!(ksig->ka.sa.sa_flags & SA_RESTART)) {
         |       ^
   arch/nds32/kernel/signal.c:319:3: note: here
     319 |   case -ERESTARTNOINTR:
         |   ^~~~
   cc1: some warnings being treated as errors

vim +677 include/asm-generic/pgtable.h

   672	
   673	static inline void __pmdp_modify_prot_commit(struct vm_area_struct *vma,
   674							unsigned long addr,
   675							pmd_t *pmdp, pmd_t pmd)
   676	{
 > 677		set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
   678	}
   679	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10682 bytes --]

Re: [PATCH 1/2] mm: add helpers pmdp_modify_prot_start/commit

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3692 bytes --]

Hi Xuefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mmotm/master]
[also build test ERROR on arm64/for-next/core arm-soc/for-next asm-generic/master linus/master v5.5-rc7 next-20200117]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Xuefeng-Wang/mm-thp-rework-the-pmd-protect-changing-flow/20200117-115821
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
   scripts/Makefile.build:57: 'arch/ia64/kernel/palinfo.ko' 'arch/ia64/kernel/mca_recovery.ko' 'arch/ia64/kernel/err_inject.ko' will not be built even though obj-m is specified.
   scripts/Makefile.build:58: You cannot use subdir-y/m to visit a module Makefile. Use obj-y/m instead.
   In file included from arch/ia64/include/asm/pgtable.h:589:0,
   from include/linux/mm.h:99,
   from arch/ia64/include/asm/uaccess.h:38,
   from include/linux/uaccess.h:11,
   from include/linux/sched/task.h:11,
   from include/linux/sched/signal.h:9,
   from arch/ia64/kernel/asm-offsets.c:10:
   include/asm-generic/pgtable.h: In function '__pmdp_modify_prot_commit':
>> include/asm-generic/pgtable.h:677:2: error: implicit declaration of function 'set_pmd_at'; did you mean
   set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
   ^~~~~~~~~~
   set_pte_at
   cc1: some warnings being treated as errors
   Makefile Module.symvers System.map arch block certs crypto drivers fs include init ipc kernel kunit lib mm modules.builtin modules.builtin.modinfo modules.order net scripts security sound source usr virt vmlinux vmlinux.bin vmlinux.gz vmlinux.o Error 1
   Target '__build' not remade because of errors.
   Makefile Module.symvers System.map arch block certs crypto drivers fs include init ipc kernel kunit lib mm modules.builtin modules.builtin.modinfo modules.order net scripts security sound source usr virt vmlinux vmlinux.bin vmlinux.gz vmlinux.o Error 2
   Target 'prepare' not remade because of errors.
   make: Makefile Module.symvers System.map arch block certs crypto drivers fs include init ipc kernel kunit lib mm modules.builtin modules.builtin.modinfo modules.order net scripts security sound source usr virt vmlinux vmlinux.bin vmlinux.gz vmlinux.o Error 2
   13 real 5 user 5 sys 80.10% cpu make prepare

vim +/set_pmd_at +677 include/asm-generic/pgtable.h

   672	
   673	static inline void __pmdp_modify_prot_commit(struct vm_area_struct *vma,
   674							unsigned long addr,
   675							pmd_t *pmdp, pmd_t pmd)
   676	{
 > 677		set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
   678	}
   679	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54862 bytes --]