[PATCH v2 0/7] Optimize mprotect for large folios

Dev Jain posted 7 patches 9 months, 2 weeks ago
arch/arm64/include/asm/pgtable.h |  10 ++
arch/arm64/mm/mmu.c              |  21 +++-
include/linux/mm.h               |   4 +-
include/linux/pgtable.h          |  42 ++++++++
mm/gup.c                         |   2 +-
mm/huge_memory.c                 |   4 +-
mm/memory.c                      |   6 +-
mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
mm/pgtable-generic.c             |  16 ++-
9 files changed, 198 insertions(+), 72 deletions(-)
[PATCH v2 0/7] Optimize mprotect for large folios
Posted by Dev Jain 9 months, 2 weeks ago
This patchset optimizes the mprotect() system call for large folios
by PTE-batching.

We use the following test cases to measure performance, mprotect()'ing
the mapped memory to read-only then read-write 40 times:

Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
pte-mapping those THPs
Test case 2: Mapping 1G of memory with 64K mTHPs
Test case 3: Mapping 1G of memory with 4K pages

Average execution time on arm64, Apple M3:
Before the patchset:
T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds

After the patchset:
T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds

Observing T1/T2 and T3 before the patchset, we also remove the regression
introduced by ptep_get() on a contpte block. And, for large folios we get
an almost 74% performance improvement.

v1->v2:
 - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the header more resilient)
 - Abridge the anon-exclusive condition (Lance Yang)

Dev Jain (7):
  mm: Refactor code in mprotect
  mm: Optimize mprotect() by batch-skipping PTEs
  mm: Add batched versions of ptep_modify_prot_start/commit
  arm64: Add batched version of ptep_modify_prot_start
  arm64: Add batched version of ptep_modify_prot_commit
  mm: Batch around can_change_pte_writable()
  mm: Optimize mprotect() through PTE-batching

 arch/arm64/include/asm/pgtable.h |  10 ++
 arch/arm64/mm/mmu.c              |  21 +++-
 include/linux/mm.h               |   4 +-
 include/linux/pgtable.h          |  42 ++++++++
 mm/gup.c                         |   2 +-
 mm/huge_memory.c                 |   4 +-
 mm/memory.c                      |   6 +-
 mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
 mm/pgtable-generic.c             |  16 ++-
 9 files changed, 198 insertions(+), 72 deletions(-)

-- 
2.30.2
Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Lance Yang 9 months, 2 weeks ago
Hey Dev,

Hmm... I also hit the same compilation errors:

In file included from ./include/linux/kasan.h:37,
                  from ./include/linux/slab.h:260,
                  from ./include/linux/crypto.h:19,
                  from arch/x86/kernel/asm-offsets.c:9:
./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
./include/linux/pgtable.h:905:15: error: implicit declaration of 
function ‘ptep_modify_prot_start’ [-Werror=implicit-function-declaration]
   905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
       |               ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h:905:15: error: incompatible types when 
assigning to type ‘pte_t’ from type ‘int’
./include/linux/pgtable.h:909:27: error: incompatible types when 
assigning to type ‘pte_t’ from type ‘int’
   909 |                 tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
       |                           ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
./include/linux/pgtable.h:925:17: error: implicit declaration of 
function ‘ptep_modify_prot_commit’ [-Werror=implicit-function-declaration]
   925 |                 ptep_modify_prot_commit(vma, addr, ptep, 
old_pte, pte);
       |                 ^~~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h: At top level:
./include/linux/pgtable.h:1360:21: error: conflicting types for 
‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long 
unsigned int,  pte_t *)’
  1360 | static inline pte_t ptep_modify_prot_start(struct 
vm_area_struct *vma,
       |                     ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h:905:15: note: previous implicit declaration of 
‘ptep_modify_prot_start’ with type ‘int()’
   905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
       |               ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h:1371:20: warning: conflicting types for 
‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long 
unsigned int,  pte_t *, pte_t,  pte_t)’
  1371 | static inline void ptep_modify_prot_commit(struct 
vm_area_struct *vma,
       |                    ^~~~~~~~~~~~~~~~~~~~~~~
./include/linux/pgtable.h:1371:20: error: static declaration of 
‘ptep_modify_prot_commit’ follows non-static declaration
./include/linux/pgtable.h:925:17: note: previous implicit declaration of 
‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, long 
unsigned int,  pte_t *, pte_t,  pte_t)’
   925 |                 ptep_modify_prot_commit(vma, addr, ptep, 
old_pte, pte);
       |                 ^~~~~~~~~~~~~~~~~~~~~~~
   CC 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/libstring.o
   CC 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/libctype.o
   CC 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/str_error_r.o
   CC 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/librbtree.o
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm-offsets.s] 
Error 1
make[1]: *** 
[/home/runner/work/mm-test-robot/mm-test-robot/linux/Makefile:1280: 
prepare0] Error 2
make[1]: *** Waiting for unfinished jobs....
   LD 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/objtool-in.o
   LINK 
/home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/objtool
make: *** [Makefile:248: __sub-make] Error 2

Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
implicit declaration errors, the architecture-independent
ptep_modify_prot_start() must be defined before modify_prot_start_ptes().

With the changes below, things work correctly now ;)

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 10cdb87ccecf..d9d6c49bb914 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct mm_struct 
*mm, unsigned long addr,
  }
  #endif

-/* See the comment for ptep_modify_prot_start */
-#ifndef modify_prot_start_ptes
-static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
-		unsigned long addr, pte_t *ptep, unsigned int nr)
-{
-	pte_t pte, tmp_pte;
-
-	pte = ptep_modify_prot_start(vma, addr, ptep);
-	while (--nr) {
-		ptep++;
-		addr += PAGE_SIZE;
-		tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
-		if (pte_dirty(tmp_pte))
-			pte = pte_mkdirty(pte);
-		if (pte_young(tmp_pte))
-			pte = pte_mkyoung(pte);
-	}
-	return pte;
-}
-#endif
-
-/* See the comment for ptep_modify_prot_commit */
-#ifndef modify_prot_commit_ptes
-static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, 
unsigned long addr,
-		pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
-{
-	for (;;) {
-		ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
-		if (--nr == 0)
-			break;
-		ptep++;
-		addr += PAGE_SIZE;
-		old_pte = pte_next_pfn(old_pte);
-		pte = pte_next_pfn(pte);
-	}
-}
-#endif
-
  /*
   * On some architectures hardware does not set page access bit when 
accessing
   * memory page, it is responsibility of software setting this bit. It 
brings
@@ -1375,6 +1337,45 @@ 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 */
+
+/* See the comment for ptep_modify_prot_start */
+#ifndef modify_prot_start_ptes
+static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
+		unsigned long addr, pte_t *ptep, unsigned int nr)
+{
+	pte_t pte, tmp_pte;
+
+	pte = ptep_modify_prot_start(vma, addr, ptep);
+	while (--nr) {
+		ptep++;
+		addr += PAGE_SIZE;
+		tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
+		if (pte_dirty(tmp_pte))
+			pte = pte_mkdirty(pte);
+		if (pte_young(tmp_pte))
+			pte = pte_mkyoung(pte);
+	}
+	return pte;
+}
+#endif
+
+/* See the comment for ptep_modify_prot_commit */
+#ifndef modify_prot_commit_ptes
+static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, 
unsigned long addr,
+		pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
+{
+	for (;;) {
+		ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
+		if (--nr == 0)
+			break;
+		ptep++;
+		addr += PAGE_SIZE;
+		old_pte = pte_next_pfn(old_pte);
+		pte = pte_next_pfn(pte);
+	}
+}
+#endif
+
  #endif /* CONFIG_MMU */

  /*
--

Thanks,
Lance

On 2025/4/29 13:23, Dev Jain wrote:
> This patchset optimizes the mprotect() system call for large folios
> by PTE-batching.
> 
> We use the following test cases to measure performance, mprotect()'ing
> the mapped memory to read-only then read-write 40 times:
> 
> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
> pte-mapping those THPs
> Test case 2: Mapping 1G of memory with 64K mTHPs
> Test case 3: Mapping 1G of memory with 4K pages
> 
> Average execution time on arm64, Apple M3:
> Before the patchset:
> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
> 
> After the patchset:
> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
> 
> Observing T1/T2 and T3 before the patchset, we also remove the regression
> introduced by ptep_get() on a contpte block. And, for large folios we get
> an almost 74% performance improvement.
> 
> v1->v2:
>   - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the header more resilient)
>   - Abridge the anon-exclusive condition (Lance Yang)
> 
> Dev Jain (7):
>    mm: Refactor code in mprotect
>    mm: Optimize mprotect() by batch-skipping PTEs
>    mm: Add batched versions of ptep_modify_prot_start/commit
>    arm64: Add batched version of ptep_modify_prot_start
>    arm64: Add batched version of ptep_modify_prot_commit
>    mm: Batch around can_change_pte_writable()
>    mm: Optimize mprotect() through PTE-batching
> 
>   arch/arm64/include/asm/pgtable.h |  10 ++
>   arch/arm64/mm/mmu.c              |  21 +++-
>   include/linux/mm.h               |   4 +-
>   include/linux/pgtable.h          |  42 ++++++++
>   mm/gup.c                         |   2 +-
>   mm/huge_memory.c                 |   4 +-
>   mm/memory.c                      |   6 +-
>   mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
>   mm/pgtable-generic.c             |  16 ++-
>   9 files changed, 198 insertions(+), 72 deletions(-)
> 

Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Dev Jain 9 months, 2 weeks ago

On 29/04/25 12:36 pm, Lance Yang wrote:
> Hey Dev,
> 
> Hmm... I also hit the same compilation errors:
> 
> In file included from ./include/linux/kasan.h:37,
>                   from ./include/linux/slab.h:260,
>                   from ./include/linux/crypto.h:19,
>                   from arch/x86/kernel/asm-offsets.c:9:
> ./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
> ./include/linux/pgtable.h:905:15: error: implicit declaration of 
> function ‘ptep_modify_prot_start’ [-Werror=implicit-function-declaration]
>    905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>        |               ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h:905:15: error: incompatible types when 
> assigning to type ‘pte_t’ from type ‘int’
> ./include/linux/pgtable.h:909:27: error: incompatible types when 
> assigning to type ‘pte_t’ from type ‘int’
>    909 |                 tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>        |                           ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
> ./include/linux/pgtable.h:925:17: error: implicit declaration of 
> function ‘ptep_modify_prot_commit’ [-Werror=implicit-function-declaration]
>    925 |                 ptep_modify_prot_commit(vma, addr, ptep, 
> old_pte, pte);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h: At top level:
> ./include/linux/pgtable.h:1360:21: error: conflicting types for 
> ‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long 
> unsigned int,  pte_t *)’
>   1360 | static inline pte_t ptep_modify_prot_start(struct 
> vm_area_struct *vma,
>        |                     ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h:905:15: note: previous implicit declaration of 
> ‘ptep_modify_prot_start’ with type ‘int()’
>    905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>        |               ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h:1371:20: warning: conflicting types for 
> ‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long 
> unsigned int,  pte_t *, pte_t,  pte_t)’
>   1371 | static inline void ptep_modify_prot_commit(struct 
> vm_area_struct *vma,
>        |                    ^~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/pgtable.h:1371:20: error: static declaration of 
> ‘ptep_modify_prot_commit’ follows non-static declaration
> ./include/linux/pgtable.h:925:17: note: previous implicit declaration of 
> ‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, long 
> unsigned int,  pte_t *, pte_t,  pte_t)’
>    925 |                 ptep_modify_prot_commit(vma, addr, ptep, 
> old_pte, pte);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~
>    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/ 
> libstring.o
>    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/ 
> libctype.o
>    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/ 
> str_error_r.o
>    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/ 
> librbtree.o
> cc1: some warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm-offsets.s] 
> Error 1
> make[1]: *** [/home/runner/work/mm-test-robot/mm-test-robot/linux/ 
> Makefile:1280: prepare0] Error 2
> make[1]: *** Waiting for unfinished jobs....
>    LD /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/ 
> objtool-in.o
>    LINK /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
> objtool/objtool
> make: *** [Makefile:248: __sub-make] Error 2
> 
> Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
> does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
> implicit declaration errors, the architecture-independent
> ptep_modify_prot_start() must be defined before modify_prot_start_ptes().
> 
> With the changes below, things work correctly now ;)

Ah thanks! My bad :(

> 
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 10cdb87ccecf..d9d6c49bb914 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct mm_struct 
> *mm, unsigned long addr,
>   }
>   #endif
> 
> -/* See the comment for ptep_modify_prot_start */
> -#ifndef modify_prot_start_ptes
> -static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
> -        unsigned long addr, pte_t *ptep, unsigned int nr)
> -{
> -    pte_t pte, tmp_pte;
> -
> -    pte = ptep_modify_prot_start(vma, addr, ptep);
> -    while (--nr) {
> -        ptep++;
> -        addr += PAGE_SIZE;
> -        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
> -        if (pte_dirty(tmp_pte))
> -            pte = pte_mkdirty(pte);
> -        if (pte_young(tmp_pte))
> -            pte = pte_mkyoung(pte);
> -    }
> -    return pte;
> -}
> -#endif
> -
> -/* See the comment for ptep_modify_prot_commit */
> -#ifndef modify_prot_commit_ptes
> -static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, 
> unsigned long addr,
> -        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
> -{
> -    for (;;) {
> -        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
> -        if (--nr == 0)
> -            break;
> -        ptep++;
> -        addr += PAGE_SIZE;
> -        old_pte = pte_next_pfn(old_pte);
> -        pte = pte_next_pfn(pte);
> -    }
> -}
> -#endif
> -
>   /*
>    * On some architectures hardware does not set page access bit when 
> accessing
>    * memory page, it is responsibility of software setting this bit. It 
> brings
> @@ -1375,6 +1337,45 @@ 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 */
> +
> +/* See the comment for ptep_modify_prot_start */
> +#ifndef modify_prot_start_ptes
> +static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
> +        unsigned long addr, pte_t *ptep, unsigned int nr)
> +{
> +    pte_t pte, tmp_pte;
> +
> +    pte = ptep_modify_prot_start(vma, addr, ptep);
> +    while (--nr) {
> +        ptep++;
> +        addr += PAGE_SIZE;
> +        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
> +        if (pte_dirty(tmp_pte))
> +            pte = pte_mkdirty(pte);
> +        if (pte_young(tmp_pte))
> +            pte = pte_mkyoung(pte);
> +    }
> +    return pte;
> +}
> +#endif
> +
> +/* See the comment for ptep_modify_prot_commit */
> +#ifndef modify_prot_commit_ptes
> +static inline void modify_prot_commit_ptes(struct vm_area_struct *vma, 
> unsigned long addr,
> +        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
> +{
> +    for (;;) {
> +        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
> +        if (--nr == 0)
> +            break;
> +        ptep++;
> +        addr += PAGE_SIZE;
> +        old_pte = pte_next_pfn(old_pte);
> +        pte = pte_next_pfn(pte);
> +    }
> +}
> +#endif
> +
>   #endif /* CONFIG_MMU */
> 
>   /*
> -- 
> 
> Thanks,
> Lance
> 
> On 2025/4/29 13:23, Dev Jain wrote:
>> This patchset optimizes the mprotect() system call for large folios
>> by PTE-batching.
>>
>> We use the following test cases to measure performance, mprotect()'ing
>> the mapped memory to read-only then read-write 40 times:
>>
>> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
>> pte-mapping those THPs
>> Test case 2: Mapping 1G of memory with 64K mTHPs
>> Test case 3: Mapping 1G of memory with 4K pages
>>
>> Average execution time on arm64, Apple M3:
>> Before the patchset:
>> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
>>
>> After the patchset:
>> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
>>
>> Observing T1/T2 and T3 before the patchset, we also remove the regression
>> introduced by ptep_get() on a contpte block. And, for large folios we get
>> an almost 74% performance improvement.
>>
>> v1->v2:
>>   - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the 
>> header more resilient)
>>   - Abridge the anon-exclusive condition (Lance Yang)
>>
>> Dev Jain (7):
>>    mm: Refactor code in mprotect
>>    mm: Optimize mprotect() by batch-skipping PTEs
>>    mm: Add batched versions of ptep_modify_prot_start/commit
>>    arm64: Add batched version of ptep_modify_prot_start
>>    arm64: Add batched version of ptep_modify_prot_commit
>>    mm: Batch around can_change_pte_writable()
>>    mm: Optimize mprotect() through PTE-batching
>>
>>   arch/arm64/include/asm/pgtable.h |  10 ++
>>   arch/arm64/mm/mmu.c              |  21 +++-
>>   include/linux/mm.h               |   4 +-
>>   include/linux/pgtable.h          |  42 ++++++++
>>   mm/gup.c                         |   2 +-
>>   mm/huge_memory.c                 |   4 +-
>>   mm/memory.c                      |   6 +-
>>   mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
>>   mm/pgtable-generic.c             |  16 ++-
>>   9 files changed, 198 insertions(+), 72 deletions(-)
>>
> 

Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Lorenzo Stoakes 9 months, 2 weeks ago
FWIW can confirm the same thing. Lance's fixes sort most of it out, but I also
get this error:

mm/mprotect.c: In function ‘can_change_ptes_writable’:
mm/mprotect.c:46:22: error: unused variable ‘page’ [-Werror=unused-variable]
   46 |         struct page *page;
      |                      ^~~~

So you also need to remove this unused variable at the stop of
can_change_ptes_writable().

Cheers, Lorenzo

On Tue, Apr 29, 2025 at 02:32:59PM +0530, Dev Jain wrote:
>
>
> On 29/04/25 12:36 pm, Lance Yang wrote:
> > Hey Dev,
> >
> > Hmm... I also hit the same compilation errors:
> >
> > In file included from ./include/linux/kasan.h:37,
> >                   from ./include/linux/slab.h:260,
> >                   from ./include/linux/crypto.h:19,
> >                   from arch/x86/kernel/asm-offsets.c:9:
> > ./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
> > ./include/linux/pgtable.h:905:15: error: implicit declaration of
> > function ‘ptep_modify_prot_start’
> > [-Werror=implicit-function-declaration]
> >    905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
> >        |               ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h:905:15: error: incompatible types when
> > assigning to type ‘pte_t’ from type ‘int’
> > ./include/linux/pgtable.h:909:27: error: incompatible types when
> > assigning to type ‘pte_t’ from type ‘int’
> >    909 |                 tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
> >        |                           ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
> > ./include/linux/pgtable.h:925:17: error: implicit declaration of
> > function ‘ptep_modify_prot_commit’
> > [-Werror=implicit-function-declaration]
> >    925 |                 ptep_modify_prot_commit(vma, addr, ptep,
> > old_pte, pte);
> >        |                 ^~~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h: At top level:
> > ./include/linux/pgtable.h:1360:21: error: conflicting types for
> > ‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long
> > unsigned int,  pte_t *)’
> >   1360 | static inline pte_t ptep_modify_prot_start(struct
> > vm_area_struct *vma,
> >        |                     ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h:905:15: note: previous implicit declaration of
> > ‘ptep_modify_prot_start’ with type ‘int()’
> >    905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
> >        |               ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h:1371:20: warning: conflicting types for
> > ‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long
> > unsigned int,  pte_t *, pte_t,  pte_t)’
> >   1371 | static inline void ptep_modify_prot_commit(struct
> > vm_area_struct *vma,
> >        |                    ^~~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/pgtable.h:1371:20: error: static declaration of
> > ‘ptep_modify_prot_commit’ follows non-static declaration
> > ./include/linux/pgtable.h:925:17: note: previous implicit declaration of
> > ‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, long
> > unsigned int,  pte_t *, pte_t,  pte_t)’
> >    925 |                 ptep_modify_prot_commit(vma, addr, ptep,
> > old_pte, pte);
> >        |                 ^~~~~~~~~~~~~~~~~~~~~~~
> >    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
> > libstring.o
> >    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
> > libctype.o
> >    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
> > str_error_r.o
> >    CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
> > librbtree.o
> > cc1: some warnings being treated as errors
> > make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm-offsets.s]
> > Error 1
> > make[1]: *** [/home/runner/work/mm-test-robot/mm-test-robot/linux/
> > Makefile:1280: prepare0] Error 2
> > make[1]: *** Waiting for unfinished jobs....
> >    LD /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
> > objtool-in.o
> >    LINK /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/
> > objtool/objtool
> > make: *** [Makefile:248: __sub-make] Error 2
> >
> > Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
> > does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
> > implicit declaration errors, the architecture-independent
> > ptep_modify_prot_start() must be defined before modify_prot_start_ptes().
> >
> > With the changes below, things work correctly now ;)
>
> Ah thanks! My bad :(
>
> >
> > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> > index 10cdb87ccecf..d9d6c49bb914 100644
> > --- a/include/linux/pgtable.h
> > +++ b/include/linux/pgtable.h
> > @@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct mm_struct
> > *mm, unsigned long addr,
> >   }
> >   #endif
> >
> > -/* See the comment for ptep_modify_prot_start */
> > -#ifndef modify_prot_start_ptes
> > -static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
> > -        unsigned long addr, pte_t *ptep, unsigned int nr)
> > -{
> > -    pte_t pte, tmp_pte;
> > -
> > -    pte = ptep_modify_prot_start(vma, addr, ptep);
> > -    while (--nr) {
> > -        ptep++;
> > -        addr += PAGE_SIZE;
> > -        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
> > -        if (pte_dirty(tmp_pte))
> > -            pte = pte_mkdirty(pte);
> > -        if (pte_young(tmp_pte))
> > -            pte = pte_mkyoung(pte);
> > -    }
> > -    return pte;
> > -}
> > -#endif
> > -
> > -/* See the comment for ptep_modify_prot_commit */
> > -#ifndef modify_prot_commit_ptes
> > -static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
> > unsigned long addr,
> > -        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
> > -{
> > -    for (;;) {
> > -        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
> > -        if (--nr == 0)
> > -            break;
> > -        ptep++;
> > -        addr += PAGE_SIZE;
> > -        old_pte = pte_next_pfn(old_pte);
> > -        pte = pte_next_pfn(pte);
> > -    }
> > -}
> > -#endif
> > -
> >   /*
> >    * On some architectures hardware does not set page access bit when
> > accessing
> >    * memory page, it is responsibility of software setting this bit. It
> > brings
> > @@ -1375,6 +1337,45 @@ 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 */
> > +
> > +/* See the comment for ptep_modify_prot_start */
> > +#ifndef modify_prot_start_ptes
> > +static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
> > +        unsigned long addr, pte_t *ptep, unsigned int nr)
> > +{
> > +    pte_t pte, tmp_pte;
> > +
> > +    pte = ptep_modify_prot_start(vma, addr, ptep);
> > +    while (--nr) {
> > +        ptep++;
> > +        addr += PAGE_SIZE;
> > +        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
> > +        if (pte_dirty(tmp_pte))
> > +            pte = pte_mkdirty(pte);
> > +        if (pte_young(tmp_pte))
> > +            pte = pte_mkyoung(pte);
> > +    }
> > +    return pte;
> > +}
> > +#endif
> > +
> > +/* See the comment for ptep_modify_prot_commit */
> > +#ifndef modify_prot_commit_ptes
> > +static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
> > unsigned long addr,
> > +        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
> > +{
> > +    for (;;) {
> > +        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
> > +        if (--nr == 0)
> > +            break;
> > +        ptep++;
> > +        addr += PAGE_SIZE;
> > +        old_pte = pte_next_pfn(old_pte);
> > +        pte = pte_next_pfn(pte);
> > +    }
> > +}
> > +#endif
> > +
> >   #endif /* CONFIG_MMU */
> >
> >   /*
> > --
> >
> > Thanks,
> > Lance
> >
> > On 2025/4/29 13:23, Dev Jain wrote:
> > > This patchset optimizes the mprotect() system call for large folios
> > > by PTE-batching.
> > >
> > > We use the following test cases to measure performance, mprotect()'ing
> > > the mapped memory to read-only then read-write 40 times:
> > >
> > > Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
> > > pte-mapping those THPs
> > > Test case 2: Mapping 1G of memory with 64K mTHPs
> > > Test case 3: Mapping 1G of memory with 4K pages
> > >
> > > Average execution time on arm64, Apple M3:
> > > Before the patchset:
> > > T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
> > >
> > > After the patchset:
> > > T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
> > >
> > > Observing T1/T2 and T3 before the patchset, we also remove the regression
> > > introduced by ptep_get() on a contpte block. And, for large folios we get
> > > an almost 74% performance improvement.
> > >
> > > v1->v2:
> > >   - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the
> > > header more resilient)
> > >   - Abridge the anon-exclusive condition (Lance Yang)
> > >
> > > Dev Jain (7):
> > >    mm: Refactor code in mprotect
> > >    mm: Optimize mprotect() by batch-skipping PTEs
> > >    mm: Add batched versions of ptep_modify_prot_start/commit
> > >    arm64: Add batched version of ptep_modify_prot_start
> > >    arm64: Add batched version of ptep_modify_prot_commit
> > >    mm: Batch around can_change_pte_writable()
> > >    mm: Optimize mprotect() through PTE-batching
> > >
> > >   arch/arm64/include/asm/pgtable.h |  10 ++
> > >   arch/arm64/mm/mmu.c              |  21 +++-
> > >   include/linux/mm.h               |   4 +-
> > >   include/linux/pgtable.h          |  42 ++++++++
> > >   mm/gup.c                         |   2 +-
> > >   mm/huge_memory.c                 |   4 +-
> > >   mm/memory.c                      |   6 +-
> > >   mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
> > >   mm/pgtable-generic.c             |  16 ++-
> > >   9 files changed, 198 insertions(+), 72 deletions(-)
> > >
> >
>
Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Dev Jain 9 months, 1 week ago

On 29/04/25 4:11 pm, Lorenzo Stoakes wrote:
> FWIW can confirm the same thing. Lance's fixes sort most of it out, but I also
> get this error:
> 
> mm/mprotect.c: In function ‘can_change_ptes_writable’:
> mm/mprotect.c:46:22: error: unused variable ‘page’ [-Werror=unused-variable]
>     46 |         struct page *page;
>        |                      ^~~~
> 
> So you also need to remove this unused variable at the stop of
> can_change_ptes_writable().

Strange that my build didn't catch this.

> 
> Cheers, Lorenzo
> 
> On Tue, Apr 29, 2025 at 02:32:59PM +0530, Dev Jain wrote:
>>
>>
>> On 29/04/25 12:36 pm, Lance Yang wrote:
>>> Hey Dev,
>>>
>>> Hmm... I also hit the same compilation errors:
>>>
>>> In file included from ./include/linux/kasan.h:37,
>>>                    from ./include/linux/slab.h:260,
>>>                    from ./include/linux/crypto.h:19,
>>>                    from arch/x86/kernel/asm-offsets.c:9:
>>> ./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
>>> ./include/linux/pgtable.h:905:15: error: implicit declaration of
>>> function ‘ptep_modify_prot_start’
>>> [-Werror=implicit-function-declaration]
>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h:905:15: error: incompatible types when
>>> assigning to type ‘pte_t’ from type ‘int’
>>> ./include/linux/pgtable.h:909:27: error: incompatible types when
>>> assigning to type ‘pte_t’ from type ‘int’
>>>     909 |                 tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>>         |                           ^~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
>>> ./include/linux/pgtable.h:925:17: error: implicit declaration of
>>> function ‘ptep_modify_prot_commit’
>>> [-Werror=implicit-function-declaration]
>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>> old_pte, pte);
>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h: At top level:
>>> ./include/linux/pgtable.h:1360:21: error: conflicting types for
>>> ‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long
>>> unsigned int,  pte_t *)’
>>>    1360 | static inline pte_t ptep_modify_prot_start(struct
>>> vm_area_struct *vma,
>>>         |                     ^~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h:905:15: note: previous implicit declaration of
>>> ‘ptep_modify_prot_start’ with type ‘int()’
>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h:1371:20: warning: conflicting types for
>>> ‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long
>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>    1371 | static inline void ptep_modify_prot_commit(struct
>>> vm_area_struct *vma,
>>>         |                    ^~~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/pgtable.h:1371:20: error: static declaration of
>>> ‘ptep_modify_prot_commit’ follows non-static declaration
>>> ./include/linux/pgtable.h:925:17: note: previous implicit declaration of
>>> ‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, long
>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>> old_pte, pte);
>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
>>> libstring.o
>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
>>> libctype.o
>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
>>> str_error_r.o
>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
>>> librbtree.o
>>> cc1: some warnings being treated as errors
>>> make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm-offsets.s]
>>> Error 1
>>> make[1]: *** [/home/runner/work/mm-test-robot/mm-test-robot/linux/
>>> Makefile:1280: prepare0] Error 2
>>> make[1]: *** Waiting for unfinished jobs....
>>>     LD /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/objtool/
>>> objtool-in.o
>>>     LINK /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/
>>> objtool/objtool
>>> make: *** [Makefile:248: __sub-make] Error 2
>>>
>>> Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
>>> does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
>>> implicit declaration errors, the architecture-independent
>>> ptep_modify_prot_start() must be defined before modify_prot_start_ptes().
>>>
>>> With the changes below, things work correctly now ;)
>>
>> Ah thanks! My bad :(
>>
>>>
>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>> index 10cdb87ccecf..d9d6c49bb914 100644
>>> --- a/include/linux/pgtable.h
>>> +++ b/include/linux/pgtable.h
>>> @@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct mm_struct
>>> *mm, unsigned long addr,
>>>    }
>>>    #endif
>>>
>>> -/* See the comment for ptep_modify_prot_start */
>>> -#ifndef modify_prot_start_ptes
>>> -static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
>>> -        unsigned long addr, pte_t *ptep, unsigned int nr)
>>> -{
>>> -    pte_t pte, tmp_pte;
>>> -
>>> -    pte = ptep_modify_prot_start(vma, addr, ptep);
>>> -    while (--nr) {
>>> -        ptep++;
>>> -        addr += PAGE_SIZE;
>>> -        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>> -        if (pte_dirty(tmp_pte))
>>> -            pte = pte_mkdirty(pte);
>>> -        if (pte_young(tmp_pte))
>>> -            pte = pte_mkyoung(pte);
>>> -    }
>>> -    return pte;
>>> -}
>>> -#endif
>>> -
>>> -/* See the comment for ptep_modify_prot_commit */
>>> -#ifndef modify_prot_commit_ptes
>>> -static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
>>> unsigned long addr,
>>> -        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>> -{
>>> -    for (;;) {
>>> -        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>> -        if (--nr == 0)
>>> -            break;
>>> -        ptep++;
>>> -        addr += PAGE_SIZE;
>>> -        old_pte = pte_next_pfn(old_pte);
>>> -        pte = pte_next_pfn(pte);
>>> -    }
>>> -}
>>> -#endif
>>> -
>>>    /*
>>>     * On some architectures hardware does not set page access bit when
>>> accessing
>>>     * memory page, it is responsibility of software setting this bit. It
>>> brings
>>> @@ -1375,6 +1337,45 @@ 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 */
>>> +
>>> +/* See the comment for ptep_modify_prot_start */
>>> +#ifndef modify_prot_start_ptes
>>> +static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
>>> +        unsigned long addr, pte_t *ptep, unsigned int nr)
>>> +{
>>> +    pte_t pte, tmp_pte;
>>> +
>>> +    pte = ptep_modify_prot_start(vma, addr, ptep);
>>> +    while (--nr) {
>>> +        ptep++;
>>> +        addr += PAGE_SIZE;
>>> +        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>> +        if (pte_dirty(tmp_pte))
>>> +            pte = pte_mkdirty(pte);
>>> +        if (pte_young(tmp_pte))
>>> +            pte = pte_mkyoung(pte);
>>> +    }
>>> +    return pte;
>>> +}
>>> +#endif
>>> +
>>> +/* See the comment for ptep_modify_prot_commit */
>>> +#ifndef modify_prot_commit_ptes
>>> +static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
>>> unsigned long addr,
>>> +        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>> +{
>>> +    for (;;) {
>>> +        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>> +        if (--nr == 0)
>>> +            break;
>>> +        ptep++;
>>> +        addr += PAGE_SIZE;
>>> +        old_pte = pte_next_pfn(old_pte);
>>> +        pte = pte_next_pfn(pte);
>>> +    }
>>> +}
>>> +#endif
>>> +
>>>    #endif /* CONFIG_MMU */
>>>
>>>    /*
>>> --
>>>
>>> Thanks,
>>> Lance
>>>
>>> On 2025/4/29 13:23, Dev Jain wrote:
>>>> This patchset optimizes the mprotect() system call for large folios
>>>> by PTE-batching.
>>>>
>>>> We use the following test cases to measure performance, mprotect()'ing
>>>> the mapped memory to read-only then read-write 40 times:
>>>>
>>>> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
>>>> pte-mapping those THPs
>>>> Test case 2: Mapping 1G of memory with 64K mTHPs
>>>> Test case 3: Mapping 1G of memory with 4K pages
>>>>
>>>> Average execution time on arm64, Apple M3:
>>>> Before the patchset:
>>>> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
>>>>
>>>> After the patchset:
>>>> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
>>>>
>>>> Observing T1/T2 and T3 before the patchset, we also remove the regression
>>>> introduced by ptep_get() on a contpte block. And, for large folios we get
>>>> an almost 74% performance improvement.
>>>>
>>>> v1->v2:
>>>>    - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the
>>>> header more resilient)
>>>>    - Abridge the anon-exclusive condition (Lance Yang)
>>>>
>>>> Dev Jain (7):
>>>>     mm: Refactor code in mprotect
>>>>     mm: Optimize mprotect() by batch-skipping PTEs
>>>>     mm: Add batched versions of ptep_modify_prot_start/commit
>>>>     arm64: Add batched version of ptep_modify_prot_start
>>>>     arm64: Add batched version of ptep_modify_prot_commit
>>>>     mm: Batch around can_change_pte_writable()
>>>>     mm: Optimize mprotect() through PTE-batching
>>>>
>>>>    arch/arm64/include/asm/pgtable.h |  10 ++
>>>>    arch/arm64/mm/mmu.c              |  21 +++-
>>>>    include/linux/mm.h               |   4 +-
>>>>    include/linux/pgtable.h          |  42 ++++++++
>>>>    mm/gup.c                         |   2 +-
>>>>    mm/huge_memory.c                 |   4 +-
>>>>    mm/memory.c                      |   6 +-
>>>>    mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
>>>>    mm/pgtable-generic.c             |  16 ++-
>>>>    9 files changed, 198 insertions(+), 72 deletions(-)
>>>>
>>>
>>

Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Lance Yang 9 months, 1 week ago

On 2025/4/30 13:42, Dev Jain wrote:
> 
> 
> On 29/04/25 4:11 pm, Lorenzo Stoakes wrote:
>> FWIW can confirm the same thing. Lance's fixes sort most of it out, 
>> but I also
>> get this error:

Good catch!

>>
>> mm/mprotect.c: In function ‘can_change_ptes_writable’:
>> mm/mprotect.c:46:22: error: unused variable ‘page’ [-Werror=unused- 
>> variable]
>>     46 |         struct page *page;
>>        |                      ^~~~
>>
>> So you also need to remove this unused variable at the stop of
>> can_change_ptes_writable().
> 
> Strange that my build didn't catch this.

Well, to catch unused variable warnings with GCC, enable stricter
checks by passing -Wunused-variable via KCFLAGS, and use 
-Werror=unused-variable
to force the build to fail if any variable is declared but unused:

make -j$(nproc) KCFLAGS="-Wunused-variable -Werror=unused-variable"

Thanks,
Lance


> 
>>
>> Cheers, Lorenzo
>>
>> On Tue, Apr 29, 2025 at 02:32:59PM +0530, Dev Jain wrote:
>>>
>>>
>>> On 29/04/25 12:36 pm, Lance Yang wrote:
>>>> Hey Dev,
>>>>
>>>> Hmm... I also hit the same compilation errors:
>>>>
>>>> In file included from ./include/linux/kasan.h:37,
>>>>                    from ./include/linux/slab.h:260,
>>>>                    from ./include/linux/crypto.h:19,
>>>>                    from arch/x86/kernel/asm-offsets.c:9:
>>>> ./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
>>>> ./include/linux/pgtable.h:905:15: error: implicit declaration of
>>>> function ‘ptep_modify_prot_start’
>>>> [-Werror=implicit-function-declaration]
>>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h:905:15: error: incompatible types when
>>>> assigning to type ‘pte_t’ from type ‘int’
>>>> ./include/linux/pgtable.h:909:27: error: incompatible types when
>>>> assigning to type ‘pte_t’ from type ‘int’
>>>>     909 |                 tmp_pte = ptep_modify_prot_start(vma, 
>>>> addr, ptep);
>>>>         |                           ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
>>>> ./include/linux/pgtable.h:925:17: error: implicit declaration of
>>>> function ‘ptep_modify_prot_commit’
>>>> [-Werror=implicit-function-declaration]
>>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>>> old_pte, pte);
>>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h: At top level:
>>>> ./include/linux/pgtable.h:1360:21: error: conflicting types for
>>>> ‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long
>>>> unsigned int,  pte_t *)’
>>>>    1360 | static inline pte_t ptep_modify_prot_start(struct
>>>> vm_area_struct *vma,
>>>>         |                     ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h:905:15: note: previous implicit 
>>>> declaration of
>>>> ‘ptep_modify_prot_start’ with type ‘int()’
>>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h:1371:20: warning: conflicting types for
>>>> ‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long
>>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>>    1371 | static inline void ptep_modify_prot_commit(struct
>>>> vm_area_struct *vma,
>>>>         |                    ^~~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/pgtable.h:1371:20: error: static declaration of
>>>> ‘ptep_modify_prot_commit’ follows non-static declaration
>>>> ./include/linux/pgtable.h:925:17: note: previous implicit 
>>>> declaration of
>>>> ‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, long
>>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>>> old_pte, pte);
>>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>> objtool/
>>>> libstring.o
>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>> objtool/
>>>> libctype.o
>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>> objtool/
>>>> str_error_r.o
>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>> objtool/
>>>> librbtree.o
>>>> cc1: some warnings being treated as errors
>>>> make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm-offsets.s]
>>>> Error 1
>>>> make[1]: *** [/home/runner/work/mm-test-robot/mm-test-robot/linux/
>>>> Makefile:1280: prepare0] Error 2
>>>> make[1]: *** Waiting for unfinished jobs....
>>>>     LD /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>> objtool/
>>>> objtool-in.o
>>>>     LINK /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/
>>>> objtool/objtool
>>>> make: *** [Makefile:248: __sub-make] Error 2
>>>>
>>>> Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
>>>> does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
>>>> implicit declaration errors, the architecture-independent
>>>> ptep_modify_prot_start() must be defined before 
>>>> modify_prot_start_ptes().
>>>>
>>>> With the changes below, things work correctly now ;)
>>>
>>> Ah thanks! My bad :(
>>>
>>>>
>>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>>> index 10cdb87ccecf..d9d6c49bb914 100644
>>>> --- a/include/linux/pgtable.h
>>>> +++ b/include/linux/pgtable.h
>>>> @@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct mm_struct
>>>> *mm, unsigned long addr,
>>>>    }
>>>>    #endif
>>>>
>>>> -/* See the comment for ptep_modify_prot_start */
>>>> -#ifndef modify_prot_start_ptes
>>>> -static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
>>>> -        unsigned long addr, pte_t *ptep, unsigned int nr)
>>>> -{
>>>> -    pte_t pte, tmp_pte;
>>>> -
>>>> -    pte = ptep_modify_prot_start(vma, addr, ptep);
>>>> -    while (--nr) {
>>>> -        ptep++;
>>>> -        addr += PAGE_SIZE;
>>>> -        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>>> -        if (pte_dirty(tmp_pte))
>>>> -            pte = pte_mkdirty(pte);
>>>> -        if (pte_young(tmp_pte))
>>>> -            pte = pte_mkyoung(pte);
>>>> -    }
>>>> -    return pte;
>>>> -}
>>>> -#endif
>>>> -
>>>> -/* See the comment for ptep_modify_prot_commit */
>>>> -#ifndef modify_prot_commit_ptes
>>>> -static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
>>>> unsigned long addr,
>>>> -        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>>> -{
>>>> -    for (;;) {
>>>> -        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>>> -        if (--nr == 0)
>>>> -            break;
>>>> -        ptep++;
>>>> -        addr += PAGE_SIZE;
>>>> -        old_pte = pte_next_pfn(old_pte);
>>>> -        pte = pte_next_pfn(pte);
>>>> -    }
>>>> -}
>>>> -#endif
>>>> -
>>>>    /*
>>>>     * On some architectures hardware does not set page access bit when
>>>> accessing
>>>>     * memory page, it is responsibility of software setting this 
>>>> bit. It
>>>> brings
>>>> @@ -1375,6 +1337,45 @@ 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 */
>>>> +
>>>> +/* See the comment for ptep_modify_prot_start */
>>>> +#ifndef modify_prot_start_ptes
>>>> +static inline pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
>>>> +        unsigned long addr, pte_t *ptep, unsigned int nr)
>>>> +{
>>>> +    pte_t pte, tmp_pte;
>>>> +
>>>> +    pte = ptep_modify_prot_start(vma, addr, ptep);
>>>> +    while (--nr) {
>>>> +        ptep++;
>>>> +        addr += PAGE_SIZE;
>>>> +        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>>> +        if (pte_dirty(tmp_pte))
>>>> +            pte = pte_mkdirty(pte);
>>>> +        if (pte_young(tmp_pte))
>>>> +            pte = pte_mkyoung(pte);
>>>> +    }
>>>> +    return pte;
>>>> +}
>>>> +#endif
>>>> +
>>>> +/* See the comment for ptep_modify_prot_commit */
>>>> +#ifndef modify_prot_commit_ptes
>>>> +static inline void modify_prot_commit_ptes(struct vm_area_struct *vma,
>>>> unsigned long addr,
>>>> +        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>>> +{
>>>> +    for (;;) {
>>>> +        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>>> +        if (--nr == 0)
>>>> +            break;
>>>> +        ptep++;
>>>> +        addr += PAGE_SIZE;
>>>> +        old_pte = pte_next_pfn(old_pte);
>>>> +        pte = pte_next_pfn(pte);
>>>> +    }
>>>> +}
>>>> +#endif
>>>> +
>>>>    #endif /* CONFIG_MMU */
>>>>
>>>>    /*
>>>> -- 
>>>>
>>>> Thanks,
>>>> Lance
>>>>
>>>> On 2025/4/29 13:23, Dev Jain wrote:
>>>>> This patchset optimizes the mprotect() system call for large folios
>>>>> by PTE-batching.
>>>>>
>>>>> We use the following test cases to measure performance, mprotect()'ing
>>>>> the mapped memory to read-only then read-write 40 times:
>>>>>
>>>>> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
>>>>> pte-mapping those THPs
>>>>> Test case 2: Mapping 1G of memory with 64K mTHPs
>>>>> Test case 3: Mapping 1G of memory with 4K pages
>>>>>
>>>>> Average execution time on arm64, Apple M3:
>>>>> Before the patchset:
>>>>> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
>>>>>
>>>>> After the patchset:
>>>>> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
>>>>>
>>>>> Observing T1/T2 and T3 before the patchset, we also remove the 
>>>>> regression
>>>>> introduced by ptep_get() on a contpte block. And, for large folios 
>>>>> we get
>>>>> an almost 74% performance improvement.
>>>>>
>>>>> v1->v2:
>>>>>    - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the
>>>>> header more resilient)
>>>>>    - Abridge the anon-exclusive condition (Lance Yang)
>>>>>
>>>>> Dev Jain (7):
>>>>>     mm: Refactor code in mprotect
>>>>>     mm: Optimize mprotect() by batch-skipping PTEs
>>>>>     mm: Add batched versions of ptep_modify_prot_start/commit
>>>>>     arm64: Add batched version of ptep_modify_prot_start
>>>>>     arm64: Add batched version of ptep_modify_prot_commit
>>>>>     mm: Batch around can_change_pte_writable()
>>>>>     mm: Optimize mprotect() through PTE-batching
>>>>>
>>>>>    arch/arm64/include/asm/pgtable.h |  10 ++
>>>>>    arch/arm64/mm/mmu.c              |  21 +++-
>>>>>    include/linux/mm.h               |   4 +-
>>>>>    include/linux/pgtable.h          |  42 ++++++++
>>>>>    mm/gup.c                         |   2 +-
>>>>>    mm/huge_memory.c                 |   4 +-
>>>>>    mm/memory.c                      |   6 +-
>>>>>    mm/mprotect.c                    | 165 +++++++++++++++++++ 
>>>>> +-----------
>>>>>    mm/pgtable-generic.c             |  16 ++-
>>>>>    9 files changed, 198 insertions(+), 72 deletions(-)
>>>>>
>>>>
>>>
> 

Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Dev Jain 9 months, 1 week ago

On 30/04/25 11:52 am, Lance Yang wrote:
> 
> 
> On 2025/4/30 13:42, Dev Jain wrote:
>>
>>
>> On 29/04/25 4:11 pm, Lorenzo Stoakes wrote:
>>> FWIW can confirm the same thing. Lance's fixes sort most of it out, 
>>> but I also
>>> get this error:
> 
> Good catch!
> 
>>>
>>> mm/mprotect.c: In function ‘can_change_ptes_writable’:
>>> mm/mprotect.c:46:22: error: unused variable ‘page’ [-Werror=unused- 
>>> variable]
>>>     46 |         struct page *page;
>>>        |                      ^~~~
>>>
>>> So you also need to remove this unused variable at the stop of
>>> can_change_ptes_writable().
>>
>> Strange that my build didn't catch this.
> 
> Well, to catch unused variable warnings with GCC, enable stricter
> checks by passing -Wunused-variable via KCFLAGS, and use -Werror=unused- 
> variable
> to force the build to fail if any variable is declared but unused:
> 
> make -j$(nproc) KCFLAGS="-Wunused-variable -Werror=unused-variable"

I thought this was the default, but thanks Lance!

> 
> Thanks,
> Lance
> 
> 
>>
>>>
>>> Cheers, Lorenzo
>>>
>>> On Tue, Apr 29, 2025 at 02:32:59PM +0530, Dev Jain wrote:
>>>>
>>>>
>>>> On 29/04/25 12:36 pm, Lance Yang wrote:
>>>>> Hey Dev,
>>>>>
>>>>> Hmm... I also hit the same compilation errors:
>>>>>
>>>>> In file included from ./include/linux/kasan.h:37,
>>>>>                    from ./include/linux/slab.h:260,
>>>>>                    from ./include/linux/crypto.h:19,
>>>>>                    from arch/x86/kernel/asm-offsets.c:9:
>>>>> ./include/linux/pgtable.h: In function ‘modify_prot_start_ptes’:
>>>>> ./include/linux/pgtable.h:905:15: error: implicit declaration of
>>>>> function ‘ptep_modify_prot_start’
>>>>> [-Werror=implicit-function-declaration]
>>>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h:905:15: error: incompatible types when
>>>>> assigning to type ‘pte_t’ from type ‘int’
>>>>> ./include/linux/pgtable.h:909:27: error: incompatible types when
>>>>> assigning to type ‘pte_t’ from type ‘int’
>>>>>     909 |                 tmp_pte = ptep_modify_prot_start(vma, 
>>>>> addr, ptep);
>>>>>         |                           ^~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h: In function ‘modify_prot_commit_ptes’:
>>>>> ./include/linux/pgtable.h:925:17: error: implicit declaration of
>>>>> function ‘ptep_modify_prot_commit’
>>>>> [-Werror=implicit-function-declaration]
>>>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>>>> old_pte, pte);
>>>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h: At top level:
>>>>> ./include/linux/pgtable.h:1360:21: error: conflicting types for
>>>>> ‘ptep_modify_prot_start’; have ‘pte_t(struct vm_area_struct *, long
>>>>> unsigned int,  pte_t *)’
>>>>>    1360 | static inline pte_t ptep_modify_prot_start(struct
>>>>> vm_area_struct *vma,
>>>>>         |                     ^~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h:905:15: note: previous implicit 
>>>>> declaration of
>>>>> ‘ptep_modify_prot_start’ with type ‘int()’
>>>>>     905 |         pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>>         |               ^~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h:1371:20: warning: conflicting types for
>>>>> ‘ptep_modify_prot_commit’; have ‘void(struct vm_area_struct *, long
>>>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>>>    1371 | static inline void ptep_modify_prot_commit(struct
>>>>> vm_area_struct *vma,
>>>>>         |                    ^~~~~~~~~~~~~~~~~~~~~~~
>>>>> ./include/linux/pgtable.h:1371:20: error: static declaration of
>>>>> ‘ptep_modify_prot_commit’ follows non-static declaration
>>>>> ./include/linux/pgtable.h:925:17: note: previous implicit 
>>>>> declaration of
>>>>> ‘ptep_modify_prot_commit’ with type ‘void(struct vm_area_struct *, 
>>>>> long
>>>>> unsigned int,  pte_t *, pte_t,  pte_t)’
>>>>>     925 |                 ptep_modify_prot_commit(vma, addr, ptep,
>>>>> old_pte, pte);
>>>>>         |                 ^~~~~~~~~~~~~~~~~~~~~~~
>>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>>> objtool/
>>>>> libstring.o
>>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>>> objtool/
>>>>> libctype.o
>>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>>> objtool/
>>>>> str_error_r.o
>>>>>     CC /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>>> objtool/
>>>>> librbtree.o
>>>>> cc1: some warnings being treated as errors
>>>>> make[2]: *** [scripts/Makefile.build:98: arch/x86/kernel/asm- 
>>>>> offsets.s]
>>>>> Error 1
>>>>> make[1]: *** [/home/runner/work/mm-test-robot/mm-test-robot/linux/
>>>>> Makefile:1280: prepare0] Error 2
>>>>> make[1]: *** Waiting for unfinished jobs....
>>>>>     LD /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/ 
>>>>> objtool/
>>>>> objtool-in.o
>>>>>     LINK /home/runner/work/mm-test-robot/mm-test-robot/linux/tools/
>>>>> objtool/objtool
>>>>> make: *** [Makefile:248: __sub-make] Error 2
>>>>>
>>>>> Well, modify_prot_start_ptes() calls ptep_modify_prot_start(), but x86
>>>>> does not define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION. To avoid
>>>>> implicit declaration errors, the architecture-independent
>>>>> ptep_modify_prot_start() must be defined before 
>>>>> modify_prot_start_ptes().
>>>>>
>>>>> With the changes below, things work correctly now ;)
>>>>
>>>> Ah thanks! My bad :(
>>>>
>>>>>
>>>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>>>> index 10cdb87ccecf..d9d6c49bb914 100644
>>>>> --- a/include/linux/pgtable.h
>>>>> +++ b/include/linux/pgtable.h
>>>>> @@ -895,44 +895,6 @@ static inline void wrprotect_ptes(struct 
>>>>> mm_struct
>>>>> *mm, unsigned long addr,
>>>>>    }
>>>>>    #endif
>>>>>
>>>>> -/* See the comment for ptep_modify_prot_start */
>>>>> -#ifndef modify_prot_start_ptes
>>>>> -static inline pte_t modify_prot_start_ptes(struct vm_area_struct 
>>>>> *vma,
>>>>> -        unsigned long addr, pte_t *ptep, unsigned int nr)
>>>>> -{
>>>>> -    pte_t pte, tmp_pte;
>>>>> -
>>>>> -    pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>> -    while (--nr) {
>>>>> -        ptep++;
>>>>> -        addr += PAGE_SIZE;
>>>>> -        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>> -        if (pte_dirty(tmp_pte))
>>>>> -            pte = pte_mkdirty(pte);
>>>>> -        if (pte_young(tmp_pte))
>>>>> -            pte = pte_mkyoung(pte);
>>>>> -    }
>>>>> -    return pte;
>>>>> -}
>>>>> -#endif
>>>>> -
>>>>> -/* See the comment for ptep_modify_prot_commit */
>>>>> -#ifndef modify_prot_commit_ptes
>>>>> -static inline void modify_prot_commit_ptes(struct vm_area_struct 
>>>>> *vma,
>>>>> unsigned long addr,
>>>>> -        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>>>> -{
>>>>> -    for (;;) {
>>>>> -        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>>>> -        if (--nr == 0)
>>>>> -            break;
>>>>> -        ptep++;
>>>>> -        addr += PAGE_SIZE;
>>>>> -        old_pte = pte_next_pfn(old_pte);
>>>>> -        pte = pte_next_pfn(pte);
>>>>> -    }
>>>>> -}
>>>>> -#endif
>>>>> -
>>>>>    /*
>>>>>     * On some architectures hardware does not set page access bit when
>>>>> accessing
>>>>>     * memory page, it is responsibility of software setting this 
>>>>> bit. It
>>>>> brings
>>>>> @@ -1375,6 +1337,45 @@ 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 */
>>>>> +
>>>>> +/* See the comment for ptep_modify_prot_start */
>>>>> +#ifndef modify_prot_start_ptes
>>>>> +static inline pte_t modify_prot_start_ptes(struct vm_area_struct 
>>>>> *vma,
>>>>> +        unsigned long addr, pte_t *ptep, unsigned int nr)
>>>>> +{
>>>>> +    pte_t pte, tmp_pte;
>>>>> +
>>>>> +    pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>> +    while (--nr) {
>>>>> +        ptep++;
>>>>> +        addr += PAGE_SIZE;
>>>>> +        tmp_pte = ptep_modify_prot_start(vma, addr, ptep);
>>>>> +        if (pte_dirty(tmp_pte))
>>>>> +            pte = pte_mkdirty(pte);
>>>>> +        if (pte_young(tmp_pte))
>>>>> +            pte = pte_mkyoung(pte);
>>>>> +    }
>>>>> +    return pte;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +/* See the comment for ptep_modify_prot_commit */
>>>>> +#ifndef modify_prot_commit_ptes
>>>>> +static inline void modify_prot_commit_ptes(struct vm_area_struct 
>>>>> *vma,
>>>>> unsigned long addr,
>>>>> +        pte_t *ptep, pte_t old_pte, pte_t pte, unsigned int nr)
>>>>> +{
>>>>> +    for (;;) {
>>>>> +        ptep_modify_prot_commit(vma, addr, ptep, old_pte, pte);
>>>>> +        if (--nr == 0)
>>>>> +            break;
>>>>> +        ptep++;
>>>>> +        addr += PAGE_SIZE;
>>>>> +        old_pte = pte_next_pfn(old_pte);
>>>>> +        pte = pte_next_pfn(pte);
>>>>> +    }
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>>    #endif /* CONFIG_MMU */
>>>>>
>>>>>    /*
>>>>> -- 
>>>>>
>>>>> Thanks,
>>>>> Lance
>>>>>
>>>>> On 2025/4/29 13:23, Dev Jain wrote:
>>>>>> This patchset optimizes the mprotect() system call for large folios
>>>>>> by PTE-batching.
>>>>>>
>>>>>> We use the following test cases to measure performance, 
>>>>>> mprotect()'ing
>>>>>> the mapped memory to read-only then read-write 40 times:
>>>>>>
>>>>>> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
>>>>>> pte-mapping those THPs
>>>>>> Test case 2: Mapping 1G of memory with 64K mTHPs
>>>>>> Test case 3: Mapping 1G of memory with 4K pages
>>>>>>
>>>>>> Average execution time on arm64, Apple M3:
>>>>>> Before the patchset:
>>>>>> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
>>>>>>
>>>>>> After the patchset:
>>>>>> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
>>>>>>
>>>>>> Observing T1/T2 and T3 before the patchset, we also remove the 
>>>>>> regression
>>>>>> introduced by ptep_get() on a contpte block. And, for large folios 
>>>>>> we get
>>>>>> an almost 74% performance improvement.
>>>>>>
>>>>>> v1->v2:
>>>>>>    - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the
>>>>>> header more resilient)
>>>>>>    - Abridge the anon-exclusive condition (Lance Yang)
>>>>>>
>>>>>> Dev Jain (7):
>>>>>>     mm: Refactor code in mprotect
>>>>>>     mm: Optimize mprotect() by batch-skipping PTEs
>>>>>>     mm: Add batched versions of ptep_modify_prot_start/commit
>>>>>>     arm64: Add batched version of ptep_modify_prot_start
>>>>>>     arm64: Add batched version of ptep_modify_prot_commit
>>>>>>     mm: Batch around can_change_pte_writable()
>>>>>>     mm: Optimize mprotect() through PTE-batching
>>>>>>
>>>>>>    arch/arm64/include/asm/pgtable.h |  10 ++
>>>>>>    arch/arm64/mm/mmu.c              |  21 +++-
>>>>>>    include/linux/mm.h               |   4 +-
>>>>>>    include/linux/pgtable.h          |  42 ++++++++
>>>>>>    mm/gup.c                         |   2 +-
>>>>>>    mm/huge_memory.c                 |   4 +-
>>>>>>    mm/memory.c                      |   6 +-
>>>>>>    mm/mprotect.c                    | 165 +++++++++++++++++++ 
>>>>>> +-----------
>>>>>>    mm/pgtable-generic.c             |  16 ++-
>>>>>>    9 files changed, 198 insertions(+), 72 deletions(-)
>>>>>>
>>>>>
>>>>
>>
> 

Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by Lorenzo Stoakes 9 months, 1 week ago
-cc namit@vmware.com

Also,

On future respins can we please drop Namit from vmware from cc list?

I am being spammed with these messages whenever I reply here:

'A custom mail flow rule created by an admin at onevmw.onmicrosoft.com has
blocked your message.  Recipient is not authorized to accept external mail'

Or ask him to fix this :)

Cheers, Lorenzo

On Tue, Apr 29, 2025 at 10:53:29AM +0530, Dev Jain wrote:
> This patchset optimizes the mprotect() system call for large folios
> by PTE-batching.
>
> We use the following test cases to measure performance, mprotect()'ing
> the mapped memory to read-only then read-write 40 times:
>
> Test case 1: Mapping 1G of memory, touching it to get PMD-THPs, then
> pte-mapping those THPs
> Test case 2: Mapping 1G of memory with 64K mTHPs
> Test case 3: Mapping 1G of memory with 4K pages
>
> Average execution time on arm64, Apple M3:
> Before the patchset:
> T1: 7.9 seconds   T2: 7.9 seconds   T3: 4.2 seconds
>
> After the patchset:
> T1: 2.1 seconds   T2: 2.2 seconds   T3: 4.2 seconds
>
> Observing T1/T2 and T3 before the patchset, we also remove the regression
> introduced by ptep_get() on a contpte block. And, for large folios we get
> an almost 74% performance improvement.
>
> v1->v2:
>  - Rebase onto mm-unstable (6ebffe676fcf: util_macros.h: make the header more resilient)
>  - Abridge the anon-exclusive condition (Lance Yang)
>
> Dev Jain (7):
>   mm: Refactor code in mprotect
>   mm: Optimize mprotect() by batch-skipping PTEs
>   mm: Add batched versions of ptep_modify_prot_start/commit
>   arm64: Add batched version of ptep_modify_prot_start
>   arm64: Add batched version of ptep_modify_prot_commit
>   mm: Batch around can_change_pte_writable()
>   mm: Optimize mprotect() through PTE-batching
>
>  arch/arm64/include/asm/pgtable.h |  10 ++
>  arch/arm64/mm/mmu.c              |  21 +++-
>  include/linux/mm.h               |   4 +-
>  include/linux/pgtable.h          |  42 ++++++++
>  mm/gup.c                         |   2 +-
>  mm/huge_memory.c                 |   4 +-
>  mm/memory.c                      |   6 +-
>  mm/mprotect.c                    | 165 ++++++++++++++++++++-----------
>  mm/pgtable-generic.c             |  16 ++-
>  9 files changed, 198 insertions(+), 72 deletions(-)
>
> --
> 2.30.2
>
Re: [PATCH v2 0/7] Optimize mprotect for large folios
Posted by David Hildenbrand 9 months, 1 week ago
On 29.04.25 13:03, Lorenzo Stoakes wrote:
> -cc namit@vmware.com

Yes, Nadav is no longer working for VMWare.

.mailmap should already include the correct mapping to the gmail address 
AFAIKS?

-- 
Cheers,

David / dhildenb