[PATCH 0/3] mm: some optimizations for prot numa

[PATCH 0/3] mm: some optimizations for prot numa

[Kefeng Wang]

Kefeng Wang (3):
  mm: mprotect: always skip dma pinned folio in prot_numa_skip()
  mm: mprotect: avoid unnecessary struct page accessing if
    pte_protnone()
  mm: huge_memory: use prot_numa_skip() for pmd folio

 mm/huge_memory.c | 21 +++++++--------------
 mm/internal.h    |  2 ++
 mm/mprotect.c    | 30 +++++++++++++++++-------------
 3 files changed, 26 insertions(+), 27 deletions(-)

-- 
2.43.0

[PATCH 1/3] mm: mprotect: always skip dma pinned folio in prot_numa_skip()

[Kefeng Wang]

If the folio(even not CoW folio) is dma pinned, it can't be
migrated, so always skip pinned folio to avoid a waste of cycles
when folios migration.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/mprotect.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/mm/mprotect.c b/mm/mprotect.c
index 113b48985834..51a28781de9d 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -136,9 +136,12 @@ static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long addr,
 	if (folio_is_zone_device(folio) || folio_test_ksm(folio))
 		goto skip;
 
-	/* Also skip shared copy-on-write pages */
-	if (is_cow_mapping(vma->vm_flags) &&
-	    (folio_maybe_dma_pinned(folio) || folio_maybe_mapped_shared(folio)))
+	/* Also skip shared copy-on-write folios */
+	if (is_cow_mapping(vma->vm_flags) && folio_maybe_mapped_shared(folio))
+		goto skip;
+
+	/* Folios are pinned and can't be migrated */
+	if (folio_maybe_dma_pinned(folio))
 		goto skip;
 
 	/*
-- 
2.43.0

[PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Kefeng Wang]

If the pte_protnone() is true, we could avoid unnecessary struct page
accessing and reduce cache footprint when scanning page tables for prot
numa, the performance test of pmbench memory accessing benchmark
should be benifit, see more commit a818f5363a0e ("autonuma: reduce cache
footprint when scanning page tables").

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/mprotect.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/mm/mprotect.c b/mm/mprotect.c
index 51a28781de9d..0f31c09c1726 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -118,18 +118,13 @@ static int mprotect_folio_pte_batch(struct folio *folio, pte_t *ptep,
 	return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr_ptes, flags);
 }
 
-static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long addr,
-			   pte_t oldpte, pte_t *pte, int target_node,
-			   struct folio *folio)
+static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
+		struct folio *folio)
 {
 	bool ret = true;
 	bool toptier;
 	int nid;
 
-	/* Avoid TLB flush if possible */
-	if (pte_protnone(oldpte))
-		goto skip;
-
 	if (!folio)
 		goto skip;
 
@@ -307,17 +302,23 @@ static long change_pte_range(struct mmu_gather *tlb,
 			struct page *page;
 			pte_t ptent;
 
+			/*
+			 * Avoid TLB flush if possible and unnecessary struct
+			 * page accessing when prot numa.
+			 */
+			if (prot_numa && pte_protnone(oldpte))
+				continue;
+
 			page = vm_normal_page(vma, addr, oldpte);
 			if (page)
 				folio = page_folio(page);
+
 			/*
 			 * Avoid trapping faults against the zero or KSM
 			 * pages. See similar comment in change_huge_pmd.
 			 */
 			if (prot_numa) {
-				int ret = prot_numa_skip(vma, addr, oldpte, pte,
-							 target_node, folio);
-				if (ret) {
+				if (prot_numa_skip(vma, target_node, folio)) {
 
 					/* determine batch to skip */
 					nr_ptes = mprotect_folio_pte_batch(folio,
-- 
2.43.0

[PATCH 3/3] mm: huge_memory: use prot_numa_skip() for pmd folio

[Kefeng Wang]

The prot_numa_skip() checks should be suitable for pmd folio too,
which helps to avoid unnecessary pmd change and folio migration
attempts.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/huge_memory.c | 21 +++++++--------------
 mm/internal.h    |  2 ++
 mm/mprotect.c    |  2 +-
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1b81680b4225..feca5a19104a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2395,8 +2395,7 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 #endif
 
 	if (prot_numa) {
-		struct folio *folio;
-		bool toptier;
+		int target_node = NUMA_NO_NODE;
 		/*
 		 * Avoid trapping faults against the zero page. The read-only
 		 * data is likely to be read-cached on the local CPU and
@@ -2408,19 +2407,13 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		if (pmd_protnone(*pmd))
 			goto unlock;
 
-		folio = pmd_folio(*pmd);
-		toptier = node_is_toptier(folio_nid(folio));
-		/*
-		 * Skip scanning top tier node if normal numa
-		 * balancing is disabled
-		 */
-		if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
-		    toptier)
-			goto unlock;
+		/* Get target node for single threaded private VMAs */
+		if (!(vma->vm_flags & VM_SHARED) &&
+		    atomic_read(&vma->vm_mm->mm_users) == 1)
+			target_node = numa_node_id();
 
-		if (folio_use_access_time(folio))
-			folio_xchg_access_time(folio,
-					       jiffies_to_msecs(jiffies));
+		if (prot_numa_skip(vma, target_node, pmd_folio(*pmd)))
+			goto unlock;
 	}
 	/*
 	 * In case prot_numa, we are under mmap_read_lock(mm). It's critical
diff --git a/mm/internal.h b/mm/internal.h
index 1561fc2ff5b8..65148cb98b9c 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1378,6 +1378,8 @@ void vunmap_range_noflush(unsigned long start, unsigned long end);
 
 void __vunmap_range_noflush(unsigned long start, unsigned long end);
 
+bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
+		struct folio *folio);
 int numa_migrate_check(struct folio *folio, struct vm_fault *vmf,
 		      unsigned long addr, int *flags, bool writable,
 		      int *last_cpupid);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 0f31c09c1726..026e7c7fa111 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -118,7 +118,7 @@ static int mprotect_folio_pte_batch(struct folio *folio, pte_t *ptep,
 	return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr_ptes, flags);
 }
 
-static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
+bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
 		struct folio *folio)
 {
 	bool ret = true;
-- 
2.43.0

Re: [PATCH 1/3] mm: mprotect: always skip dma pinned folio in prot_numa_skip()

[Sidhartha Kumar]

On 10/13/25 8:15 AM, Kefeng Wang wrote:
> If the folio(even not CoW folio) is dma pinned, it can't be
> migrated, so always skip pinned folio to avoid a waste of cycles
> when folios migration.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>

> ---
>   mm/mprotect.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 113b48985834..51a28781de9d 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -136,9 +136,12 @@ static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long addr,
>   	if (folio_is_zone_device(folio) || folio_test_ksm(folio))
>   		goto skip;
>   
> -	/* Also skip shared copy-on-write pages */
> -	if (is_cow_mapping(vma->vm_flags) &&
> -	    (folio_maybe_dma_pinned(folio) || folio_maybe_mapped_shared(folio)))
> +	/* Also skip shared copy-on-write folios */
> +	if (is_cow_mapping(vma->vm_flags) && folio_maybe_mapped_shared(folio))
> +		goto skip;
> +
> +	/* Folios are pinned and can't be migrated */
> +	if (folio_maybe_dma_pinned(folio))
>   		goto skip;
>   
>   	/*

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Sidhartha Kumar]

On 10/13/25 8:15 AM, Kefeng Wang wrote:
> If the pte_protnone() is true, we could avoid unnecessary struct page
> accessing and reduce cache footprint when scanning page tables for prot
> numa, the performance test of pmbench memory accessing benchmark
> should be benifit, see more commit a818f5363a0e ("autonuma: reduce cache
> footprint when scanning page tables").
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>

> ---
>   mm/mprotect.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 51a28781de9d..0f31c09c1726 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -118,18 +118,13 @@ static int mprotect_folio_pte_batch(struct folio *folio, pte_t *ptep,
>   	return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr_ptes, flags);
>   }
>   
> -static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long addr,
> -			   pte_t oldpte, pte_t *pte, int target_node,
> -			   struct folio *folio)
> +static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
> +		struct folio *folio)
>   {
>   	bool ret = true;
>   	bool toptier;
>   	int nid;
>   
> -	/* Avoid TLB flush if possible */
> -	if (pte_protnone(oldpte))
> -		goto skip;
> -
>   	if (!folio)
>   		goto skip;
>   
> @@ -307,17 +302,23 @@ static long change_pte_range(struct mmu_gather *tlb,
>   			struct page *page;
>   			pte_t ptent;
>   
> +			/*
> +			 * Avoid TLB flush if possible and unnecessary struct
> +			 * page accessing when prot numa.
> +			 */
> +			if (prot_numa && pte_protnone(oldpte))
> +				continue;
> +
>   			page = vm_normal_page(vma, addr, oldpte);
>   			if (page)
>   				folio = page_folio(page);
> +
>   			/*
>   			 * Avoid trapping faults against the zero or KSM
>   			 * pages. See similar comment in change_huge_pmd.
>   			 */
>   			if (prot_numa) {
> -				int ret = prot_numa_skip(vma, addr, oldpte, pte,
> -							 target_node, folio);
> -				if (ret) {
> +				if (prot_numa_skip(vma, target_node, folio)) {
>   
>   					/* determine batch to skip */
>   					nr_ptes = mprotect_folio_pte_batch(folio,

Re: [PATCH 3/3] mm: huge_memory: use prot_numa_skip() for pmd folio

[Sidhartha Kumar]

On 10/13/25 8:15 AM, Kefeng Wang wrote:
> The prot_numa_skip() checks should be suitable for pmd folio too,
> which helps to avoid unnecessary pmd change and folio migration
> attempts.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>

> ---
>   mm/huge_memory.c | 21 +++++++--------------
>   mm/internal.h    |  2 ++
>   mm/mprotect.c    |  2 +-
>   3 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 1b81680b4225..feca5a19104a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2395,8 +2395,7 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   #endif
>   
>   	if (prot_numa) {
> -		struct folio *folio;
> -		bool toptier;
> +		int target_node = NUMA_NO_NODE;
>   		/*
>   		 * Avoid trapping faults against the zero page. The read-only
>   		 * data is likely to be read-cached on the local CPU and
> @@ -2408,19 +2407,13 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   		if (pmd_protnone(*pmd))
>   			goto unlock;
>   
> -		folio = pmd_folio(*pmd);
> -		toptier = node_is_toptier(folio_nid(folio));
> -		/*
> -		 * Skip scanning top tier node if normal numa
> -		 * balancing is disabled
> -		 */
> -		if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
> -		    toptier)
> -			goto unlock;
> +		/* Get target node for single threaded private VMAs */
> +		if (!(vma->vm_flags & VM_SHARED) &&
> +		    atomic_read(&vma->vm_mm->mm_users) == 1)
> +			target_node = numa_node_id();
>   
> -		if (folio_use_access_time(folio))
> -			folio_xchg_access_time(folio,
> -					       jiffies_to_msecs(jiffies));
> +		if (prot_numa_skip(vma, target_node, pmd_folio(*pmd)))
> +			goto unlock;
>   	}
>   	/*
>   	 * In case prot_numa, we are under mmap_read_lock(mm). It's critical
> diff --git a/mm/internal.h b/mm/internal.h
> index 1561fc2ff5b8..65148cb98b9c 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1378,6 +1378,8 @@ void vunmap_range_noflush(unsigned long start, unsigned long end);
>   
>   void __vunmap_range_noflush(unsigned long start, unsigned long end);
>   
> +bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
> +		struct folio *folio);
>   int numa_migrate_check(struct folio *folio, struct vm_fault *vmf,
>   		      unsigned long addr, int *flags, bool writable,
>   		      int *last_cpupid);
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 0f31c09c1726..026e7c7fa111 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -118,7 +118,7 @@ static int mprotect_folio_pte_batch(struct folio *folio, pte_t *ptep,
>   	return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr_ptes, flags);
>   }
>   
> -static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
> +bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
>   		struct folio *folio)
>   {
>   	bool ret = true;

Re: [PATCH 1/3] mm: mprotect: always skip dma pinned folio in prot_numa_skip()

[David Hildenbrand]

On 13.10.25 14:15, Kefeng Wang wrote:
> If the folio(even not CoW folio) is dma pinned, it can't be
> migrated, so always skip pinned folio to avoid a waste of cycles
> when folios migration.

Right. It's a historical artifact from pre-anon-exclusive days.

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[David Hildenbrand]

On 13.10.25 14:15, Kefeng Wang wrote:
> If the pte_protnone() is true, we could avoid unnecessary struct page
> accessing and reduce cache footprint when scanning page tables for prot
> numa, the performance test of pmbench memory accessing benchmark
> should be benifit, see more commit a818f5363a0e ("autonuma: reduce cache
> footprint when scanning page tables").
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   mm/mprotect.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 51a28781de9d..0f31c09c1726 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -118,18 +118,13 @@ static int mprotect_folio_pte_batch(struct folio *folio, pte_t *ptep,
>   	return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr_ptes, flags);
>   }
>   
> -static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long addr,
> -			   pte_t oldpte, pte_t *pte, int target_node,
> -			   struct folio *folio)
> +static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
> +		struct folio *folio)
>   {
>   	bool ret = true;
>   	bool toptier;
>   	int nid;
>   
> -	/* Avoid TLB flush if possible */
> -	if (pte_protnone(oldpte))
> -		goto skip;
> -
>   	if (!folio)
>   		goto skip;
>   
> @@ -307,17 +302,23 @@ static long change_pte_range(struct mmu_gather *tlb,
>   			struct page *page;
>   			pte_t ptent;
>   
> +			/*
> +			 * Avoid TLB flush if possible and unnecessary struct
> +			 * page accessing when prot numa.
> +			 */

I think we should just simplify to

"/* Already in the desired state. */"

or sth. like that. No need to mention the struct-page implementation 
details in this comment. Also, I don't think there is a need to mention 
the TLB flush when it's really in the desired state already.


> +			if (prot_numa && pte_protnone(oldpte))
> +				continue;
> +
>   			page = vm_normal_page(vma, addr, oldpte);
>   			if (page)
>   				folio = page_folio(page);

I could have sworn we discussed that while fixing the prot_numa_skip() 
fallout.

-- 
Cheers

David / dhildenb

Re: [PATCH 3/3] mm: huge_memory: use prot_numa_skip() for pmd folio

[David Hildenbrand]

On 13.10.25 14:15, Kefeng Wang wrote:
> The prot_numa_skip() checks should be suitable for pmd folio too,
> which helps to avoid unnecessary pmd change and folio migration
> attempts.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   mm/huge_memory.c | 21 +++++++--------------
>   mm/internal.h    |  2 ++
>   mm/mprotect.c    |  2 +-
>   3 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 1b81680b4225..feca5a19104a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2395,8 +2395,7 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   #endif
>   
>   	if (prot_numa) {
> -		struct folio *folio;
> -		bool toptier;
> +		int target_node = NUMA_NO_NODE;
>   		/*
>   		 * Avoid trapping faults against the zero page. The read-only
>   		 * data is likely to be read-cached on the local CPU and
> @@ -2408,19 +2407,13 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>   		if (pmd_protnone(*pmd))
>   			goto unlock;
>   
> -		folio = pmd_folio(*pmd);
> -		toptier = node_is_toptier(folio_nid(folio));
> -		/*
> -		 * Skip scanning top tier node if normal numa
> -		 * balancing is disabled
> -		 */
> -		if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
> -		    toptier)
> -			goto unlock;
> +		/* Get target node for single threaded private VMAs */
> +		if (!(vma->vm_flags & VM_SHARED) &&
> +		    atomic_read(&vma->vm_mm->mm_users) == 1)
> +			target_node = numa_node_id();

Wondering if we should move that hunk into prot_numa_skip() as well.

I'd assume numa_node_id() is not particularly expensive?

While at it, I think we should then rename prot_numa_skip() to
prot_numa_skip_folio() or better "folio_skip_prot_numa()", making folio 
the first parameter.

-- 
Cheers

David / dhildenb

Re: [PATCH 1/3] mm: mprotect: always skip dma pinned folio in prot_numa_skip()

[Lance Yang]

On 2025/10/13 20:15, Kefeng Wang wrote:
> If the folio(even not CoW folio) is dma pinned, it can't be
> migrated, so always skip pinned folio to avoid a waste of cycles
> when folios migration.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---

LGTM.

Reviewed-by: Lance Yang <lance.yang@linux.dev>

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Kefeng Wang]

On 2025/10/13 23:53, David Hildenbrand wrote:
> On 13.10.25 14:15, Kefeng Wang wrote:
>> If the pte_protnone() is true, we could avoid unnecessary struct page
>> accessing and reduce cache footprint when scanning page tables for prot
>> numa, the performance test of pmbench memory accessing benchmark
>> should be benifit, see more commit a818f5363a0e ("autonuma: reduce cache
>> footprint when scanning page tables").
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/mprotect.c | 21 +++++++++++----------
>>   1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/mprotect.c b/mm/mprotect.c
>> index 51a28781de9d..0f31c09c1726 100644
>> --- a/mm/mprotect.c
>> +++ b/mm/mprotect.c
>> @@ -118,18 +118,13 @@ static int mprotect_folio_pte_batch(struct folio 
>> *folio, pte_t *ptep,
>>       return folio_pte_batch_flags(folio, NULL, ptep, &pte, 
>> max_nr_ptes, flags);
>>   }
>> -static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long 
>> addr,
>> -               pte_t oldpte, pte_t *pte, int target_node,
>> -               struct folio *folio)
>> +static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
>> +        struct folio *folio)
>>   {
>>       bool ret = true;
>>       bool toptier;
>>       int nid;
>> -    /* Avoid TLB flush if possible */
>> -    if (pte_protnone(oldpte))
>> -        goto skip;
>> -
>>       if (!folio)
>>           goto skip;
>> @@ -307,17 +302,23 @@ static long change_pte_range(struct mmu_gather 
>> *tlb,
>>               struct page *page;
>>               pte_t ptent;
>> +            /*
>> +             * Avoid TLB flush if possible and unnecessary struct
>> +             * page accessing when prot numa.
>> +             */
> 
> I think we should just simplify to
> 
> "/* Already in the desired state. */"
> 
> or sth. like that. No need to mention the struct-page implementation 
> details in this comment. Also, I don't think there is a need to mention 
> the TLB flush when it's really in the desired state already.

Sure.

> 
> 
>> +            if (prot_numa && pte_protnone(oldpte))
>> +                continue;
>> +
>>               page = vm_normal_page(vma, addr, oldpte);
>>               if (page)
>>                   folio = page_folio(page);
> 
> I could have sworn we discussed that while fixing the prot_numa_skip() 
> fallout.

I'm not follow the thread, but we found that vm_normal_page does
introduce regression for mprotect benchmark(libMicro) with
this vm_normal_page().

Re: [PATCH 3/3] mm: huge_memory: use prot_numa_skip() for pmd folio

[Kefeng Wang]

On 2025/10/13 23:58, David Hildenbrand wrote:
> On 13.10.25 14:15, Kefeng Wang wrote:
>> The prot_numa_skip() checks should be suitable for pmd folio too,
>> which helps to avoid unnecessary pmd change and folio migration
>> attempts.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/huge_memory.c | 21 +++++++--------------
>>   mm/internal.h    |  2 ++
>>   mm/mprotect.c    |  2 +-
>>   3 files changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 1b81680b4225..feca5a19104a 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2395,8 +2395,7 @@ int change_huge_pmd(struct mmu_gather *tlb, 
>> struct vm_area_struct *vma,
>>   #endif
>>       if (prot_numa) {
>> -        struct folio *folio;
>> -        bool toptier;
>> +        int target_node = NUMA_NO_NODE;
>>           /*
>>            * Avoid trapping faults against the zero page. The read-only
>>            * data is likely to be read-cached on the local CPU and
>> @@ -2408,19 +2407,13 @@ int change_huge_pmd(struct mmu_gather *tlb, 
>> struct vm_area_struct *vma,
>>           if (pmd_protnone(*pmd))
>>               goto unlock;
>> -        folio = pmd_folio(*pmd);
>> -        toptier = node_is_toptier(folio_nid(folio));
>> -        /*
>> -         * Skip scanning top tier node if normal numa
>> -         * balancing is disabled
>> -         */
>> -        if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
>> -            toptier)
>> -            goto unlock;
>> +        /* Get target node for single threaded private VMAs */
>> +        if (!(vma->vm_flags & VM_SHARED) &&
>> +            atomic_read(&vma->vm_mm->mm_users) == 1)
>> +            target_node = numa_node_id();
> 
> Wondering if we should move that hunk into prot_numa_skip() as well.
> 
> I'd assume numa_node_id() is not particularly expensive?

There is a atomic read, it may be expensive for 512 times atomic read
for change_pte_range().

> 
> While at it, I think we should then rename prot_numa_skip() to
> prot_numa_skip_folio() or better "folio_skip_prot_numa()", making folio 
> the first parameter.
> 

OK, will change it.

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[David Hildenbrand]

>>
>>> +            if (prot_numa && pte_protnone(oldpte))
>>> +                continue;
>>> +
>>>                page = vm_normal_page(vma, addr, oldpte);
>>>                if (page)
>>>                    folio = page_folio(page);
>>
>> I could have sworn we discussed that while fixing the prot_numa_skip()
>> fallout.
> 
> I'm not follow the thread, but we found that vm_normal_page does
> introduce regression for mprotect benchmark(libMicro) with
> this vm_normal_page().
> 

Right, I raised it here:

https://lkml.kernel.org/r/aa496798-5ac6-4cb0-bdc2-91515172e935@redhat.com

I questioned how relevant it would be in practice.

I'm surprised it shows up in a mprotect() benchmark: mprotect() itself 
would never be able to set MM_CP_PROT_NUMA, so the code wold not 
actually be executed.

-- 
Cheers

David / dhildenb

Re: [PATCH 3/3] mm: huge_memory: use prot_numa_skip() for pmd folio

[David Hildenbrand]

On 14.10.25 08:10, Kefeng Wang wrote:
> 
> 
> On 2025/10/13 23:58, David Hildenbrand wrote:
>> On 13.10.25 14:15, Kefeng Wang wrote:
>>> The prot_numa_skip() checks should be suitable for pmd folio too,
>>> which helps to avoid unnecessary pmd change and folio migration
>>> attempts.
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>    mm/huge_memory.c | 21 +++++++--------------
>>>    mm/internal.h    |  2 ++
>>>    mm/mprotect.c    |  2 +-
>>>    3 files changed, 10 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 1b81680b4225..feca5a19104a 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -2395,8 +2395,7 @@ int change_huge_pmd(struct mmu_gather *tlb,
>>> struct vm_area_struct *vma,
>>>    #endif
>>>        if (prot_numa) {
>>> -        struct folio *folio;
>>> -        bool toptier;
>>> +        int target_node = NUMA_NO_NODE;
>>>            /*
>>>             * Avoid trapping faults against the zero page. The read-only
>>>             * data is likely to be read-cached on the local CPU and
>>> @@ -2408,19 +2407,13 @@ int change_huge_pmd(struct mmu_gather *tlb,
>>> struct vm_area_struct *vma,
>>>            if (pmd_protnone(*pmd))
>>>                goto unlock;
>>> -        folio = pmd_folio(*pmd);
>>> -        toptier = node_is_toptier(folio_nid(folio));
>>> -        /*
>>> -         * Skip scanning top tier node if normal numa
>>> -         * balancing is disabled
>>> -         */
>>> -        if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
>>> -            toptier)
>>> -            goto unlock;
>>> +        /* Get target node for single threaded private VMAs */
>>> +        if (!(vma->vm_flags & VM_SHARED) &&
>>> +            atomic_read(&vma->vm_mm->mm_users) == 1)
>>> +            target_node = numa_node_id();
>>
>> Wondering if we should move that hunk into prot_numa_skip() as well.
>>
>> I'd assume numa_node_id() is not particularly expensive?
> 
> There is a atomic read, it may be expensive for 512 times atomic read
> for change_pte_range().

Note that atomic reads are usually just ordinary reads.

IIRC, the relevant code is

	cpu_to_node(raw_smp_processor_id())

And raw_smp_processor_id() is mostly either "current_thread_info()->cpu" 
or "this_cpu_read(cpu_number)".

But yeah, we can leave it as is for now and cache it in the caller.

-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Kefeng Wang]

On 2025/10/14 15:16, David Hildenbrand wrote:
>>>
>>>> +            if (prot_numa && pte_protnone(oldpte))
>>>> +                continue;
>>>> +
>>>>                page = vm_normal_page(vma, addr, oldpte);
>>>>                if (page)
>>>>                    folio = page_folio(page);
>>>
>>> I could have sworn we discussed that while fixing the prot_numa_skip()
>>> fallout.
>>
>> I'm not follow the thread, but we found that vm_normal_page does
>> introduce regression for mprotect benchmark(libMicro) with
>> this vm_normal_page().
>>
> 
> Right, I raised it here:
> 
> https://lkml.kernel.org/r/aa496798-5ac6-4cb0-bdc2-91515172e935@redhat.com

Thanks for the links, let's fix it now ;)

> 
> I questioned how relevant it would be in practice.
> 
> I'm surprised it shows up in a mprotect() benchmark: mprotect() itself 
> would never be able to set MM_CP_PROT_NUMA, so the code wold not 
> actually be executed.
> 

Sorry, my description is very clear, the regression is not about prot
numa, I mean the vm_normal_page does introduce some regression when
mprotect benchmark in libMicro, before cac1db8c3aad （"mm: optimize
mprotect() by PTE batching"), we only call vm_normal_page in
can_change_pte_writable(), but now it is unconditional called and
10% regression in some libMicro mprotect benchmark.

We could add limit to call of vm_normal_page， eg，cp_flags
check, and using pte_batch_hint, but it is unclear whether it is worth
doing so.

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[David Hildenbrand]

On 14.10.25 10:02, Kefeng Wang wrote:
> 
> 
> On 2025/10/14 15:16, David Hildenbrand wrote:
>>>>
>>>>> +            if (prot_numa && pte_protnone(oldpte))
>>>>> +                continue;
>>>>> +
>>>>>                 page = vm_normal_page(vma, addr, oldpte);
>>>>>                 if (page)
>>>>>                     folio = page_folio(page);
>>>>
>>>> I could have sworn we discussed that while fixing the prot_numa_skip()
>>>> fallout.
>>>
>>> I'm not follow the thread, but we found that vm_normal_page does
>>> introduce regression for mprotect benchmark(libMicro) with
>>> this vm_normal_page().
>>>
>>
>> Right, I raised it here:
>>
>> https://lkml.kernel.org/r/aa496798-5ac6-4cb0-bdc2-91515172e935@redhat.com
> 
> Thanks for the links, let's fix it now ;)
> 
>>
>> I questioned how relevant it would be in practice.
>>
>> I'm surprised it shows up in a mprotect() benchmark: mprotect() itself
>> would never be able to set MM_CP_PROT_NUMA, so the code wold not
>> actually be executed.
>>
> 
> Sorry, my description is very clear, the regression is not about prot
> numa, I mean the vm_normal_page does introduce some regression when
> mprotect benchmark in libMicro, before cac1db8c3aad （"mm: optimize
> mprotect() by PTE batching"), we only call vm_normal_page in
> can_change_pte_writable(), but now it is unconditional called and
> 10% regression in some libMicro mprotect benchmark.

Right, I think we discussed that as well at some point, and possible 
ways to optimize if we ever have to. We don't have to optimize for each 
and every microbenchmark that heavily, though.

-- 
Cheers

David / dhildenb

Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Kefeng Wang]

On 2025/10/14 16:56, David Hildenbrand wrote:
> On 14.10.25 10:02, Kefeng Wang wrote:
>>
>>
>> On 2025/10/14 15:16, David Hildenbrand wrote:
>>>>>
>>>>>> +            if (prot_numa && pte_protnone(oldpte))
>>>>>> +                continue;
>>>>>> +
>>>>>>                 page = vm_normal_page(vma, addr, oldpte);
>>>>>>                 if (page)
>>>>>>                     folio = page_folio(page);
>>>>>
>>>>> I could have sworn we discussed that while fixing the prot_numa_skip()
>>>>> fallout.
>>>>
>>>> I'm not follow the thread, but we found that vm_normal_page does
>>>> introduce regression for mprotect benchmark(libMicro) with
>>>> this vm_normal_page().
>>>>
>>>
>>> Right, I raised it here:
>>>
>>> https://lkml.kernel.org/r/aa496798-5ac6-4cb0- 
>>> bdc2-91515172e935@redhat.com
>>
>> Thanks for the links, let's fix it now ;)
>>
>>>
>>> I questioned how relevant it would be in practice.
>>>
>>> I'm surprised it shows up in a mprotect() benchmark: mprotect() itself
>>> would never be able to set MM_CP_PROT_NUMA, so the code wold not
>>> actually be executed.
>>>
>>
>> Sorry, my description is very clear, the regression is not about prot
>> numa, I mean the vm_normal_page does introduce some regression when
>> mprotect benchmark in libMicro, before cac1db8c3aad （"mm: optimize
>> mprotect() by PTE batching"), we only call vm_normal_page in
>> can_change_pte_writable(), but now it is unconditional called and
>> 10% regression in some libMicro mprotect benchmark.
> 
> Right, I think we discussed that as well at some point, and possible 
> ways to optimize if we ever have to. We don't have to optimize for each 
> and every microbenchmark that heavily, though.
> 

OK, let's do it if real benchmark hint the issue.