[1/3] mm/mempolicy: Use the already fetched local variable

  Avoid doing a per cpu read and use the local variable thisnid. IMHO
this also makes the code more readable.

Signed-off-by: Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org>
Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 mm/mempolicy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Sat, 17 Feb 2024 01:31:33 -0600 Donet Tom <donettom@linux.ibm.com> wrote:

> Avoid doing a per cpu read and use the local variable thisnid. IMHO
> this also makes the code more readable.
> 
> ...
>
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>  		if (node_isset(curnid, pol->nodes))
>  			goto out;
>  		z = first_zones_zonelist(
> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
> +				node_zonelist(thisnid, GFP_HIGHUSER),
>  				gfp_zone(GFP_HIGHUSER),
>  				&pol->nodes);
>  		polnid = zone_to_nid(z->zone);

	int thisnid = cpu_to_node(thiscpu);

Is there any dofference between numa_node_id() and
cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
using one here and not the other?
  

On 2/19/24 03:08, Andrew Morton wrote:
> On Sat, 17 Feb 2024 01:31:33 -0600 Donet Tom <donettom@linux.ibm.com> wrote:
>
>> Avoid doing a per cpu read and use the local variable thisnid. IMHO
>> this also makes the code more readable.
>>
>> ...
>>
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>   		if (node_isset(curnid, pol->nodes))
>>   			goto out;
>>   		z = first_zones_zonelist(
>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>   				gfp_zone(GFP_HIGHUSER),
>>   				&pol->nodes);
>>   		polnid = zone_to_nid(z->zone);
> 	int thisnid = cpu_to_node(thiscpu);
>
> Is there any dofference between numa_node_id() and
> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
> using one here and not the other?

Hi Andrew

Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.

Thanks
Donet Tom

>
  

On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:

> >> --- a/mm/mempolicy.c
> >> +++ b/mm/mempolicy.c
> >> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
> >>   		if (node_isset(curnid, pol->nodes))
> >>   			goto out;
> >>   		z = first_zones_zonelist(
> >> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
> >> +				node_zonelist(thisnid, GFP_HIGHUSER),
> >>   				gfp_zone(GFP_HIGHUSER),
> >>   				&pol->nodes);
> >>   		polnid = zone_to_nid(z->zone);
> > 	int thisnid = cpu_to_node(thiscpu);
> >
> > Is there any dofference between numa_node_id() and
> > cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
> > using one here and not the other?
> 
> Hi Andrew
> 
> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.

Sure, but mine was a broader thought: why do we have both?  Is one
preferable and if so why?
  

On 2/20/24 6:51 AM, Andrew Morton wrote:
> On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
> 
>>>> --- a/mm/mempolicy.c
>>>> +++ b/mm/mempolicy.c
>>>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>>>   		if (node_isset(curnid, pol->nodes))
>>>>   			goto out;
>>>>   		z = first_zones_zonelist(
>>>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>>>   				gfp_zone(GFP_HIGHUSER),
>>>>   				&pol->nodes);
>>>>   		polnid = zone_to_nid(z->zone);
>>> 	int thisnid = cpu_to_node(thiscpu);
>>>
>>> Is there any dofference between numa_node_id() and
>>> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
>>> using one here and not the other?
>>
>> Hi Andrew
>>
>> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
>> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.
> 
> Sure, but mine was a broader thought: why do we have both?  Is one
> preferable and if so why?

IIUC these are two helpers to fetch current numa node id. and either of them can be used based on need. The default implementation shows the details.
(One small difference is numa_node_id() can use optimized per cpu reader because it is fetching the per cpu variable of the currently running cpu.)

#ifndef numa_node_id
/* Returns the number of the current Node. */
static inline int numa_node_id(void)
{
	return raw_cpu_read(numa_node);
}
#endif

#ifndef cpu_to_node
static inline int cpu_to_node(int cpu)
{
	return per_cpu(numa_node, cpu);
}
#endif

In mpol_misplaced function, we need the cpu details because we are using that in other place (should_numa_migreate_memory()). So it makes it easy
to use cpu_to_node(thiscpu) instead of numa_node_id(). 

-aneesh
  

"Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:

> On 2/20/24 6:51 AM, Andrew Morton wrote:
>> On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>> 
>>>>> --- a/mm/mempolicy.c
>>>>> +++ b/mm/mempolicy.c
>>>>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>>>>   		if (node_isset(curnid, pol->nodes))
>>>>>   			goto out;
>>>>>   		z = first_zones_zonelist(
>>>>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>>>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>>>>   				gfp_zone(GFP_HIGHUSER),
>>>>>   				&pol->nodes);
>>>>>   		polnid = zone_to_nid(z->zone);
>>>> 	int thisnid = cpu_to_node(thiscpu);
>>>>
>>>> Is there any dofference between numa_node_id() and
>>>> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
>>>> using one here and not the other?
>>>
>>> Hi Andrew
>>>
>>> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
>>> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.
>> 
>> Sure, but mine was a broader thought: why do we have both?  Is one
>> preferable and if so why?
>
> IIUC these are two helpers to fetch current numa node id. and either of them can be used based on need. The default implementation shows the details.
> (One small difference is numa_node_id() can use optimized per cpu reader because it is fetching the per cpu variable of the currently running cpu.)
>
> #ifndef numa_node_id
> /* Returns the number of the current Node. */
> static inline int numa_node_id(void)
> {
> 	return raw_cpu_read(numa_node);
> }
> #endif
>
> #ifndef cpu_to_node
> static inline int cpu_to_node(int cpu)
> {
> 	return per_cpu(numa_node, cpu);
> }
> #endif
>
> In mpol_misplaced function, we need the cpu details because we are using that in other place (should_numa_migreate_memory()). So it makes it easy
> to use cpu_to_node(thiscpu) instead of numa_node_id(). 

IIUC, numa_node_id() is faster than cpu_to_node(thiscpu), even if we
have thiscpu already.  cpu_to_node() is mainly used to get the node of
NOT current CPU.  So, IMHO, we should only use numa_node_id() in this
function.

--
Best Regards,
Huang, Ying
  

On 2/20/24 11:55 AM, Huang, Ying wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:
> 
>> On 2/20/24 6:51 AM, Andrew Morton wrote:
>>> On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>>>
>>>>>> --- a/mm/mempolicy.c
>>>>>> +++ b/mm/mempolicy.c
>>>>>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>>>>>   		if (node_isset(curnid, pol->nodes))
>>>>>>   			goto out;
>>>>>>   		z = first_zones_zonelist(
>>>>>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>>>>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>>>>>   				gfp_zone(GFP_HIGHUSER),
>>>>>>   				&pol->nodes);
>>>>>>   		polnid = zone_to_nid(z->zone);
>>>>> 	int thisnid = cpu_to_node(thiscpu);
>>>>>
>>>>> Is there any dofference between numa_node_id() and
>>>>> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
>>>>> using one here and not the other?
>>>>
>>>> Hi Andrew
>>>>
>>>> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
>>>> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.
>>>
>>> Sure, but mine was a broader thought: why do we have both?  Is one
>>> preferable and if so why?
>>
>> IIUC these are two helpers to fetch current numa node id. and either of them can be used based on need. The default implementation shows the details.
>> (One small difference is numa_node_id() can use optimized per cpu reader because it is fetching the per cpu variable of the currently running cpu.)
>>
>> #ifndef numa_node_id
>> /* Returns the number of the current Node. */
>> static inline int numa_node_id(void)
>> {
>> 	return raw_cpu_read(numa_node);
>> }
>> #endif
>>
>> #ifndef cpu_to_node
>> static inline int cpu_to_node(int cpu)
>> {
>> 	return per_cpu(numa_node, cpu);
>> }
>> #endif
>>
>> In mpol_misplaced function, we need the cpu details because we are using that in other place (should_numa_migreate_memory()). So it makes it easy
>> to use cpu_to_node(thiscpu) instead of numa_node_id(). 
> 
> IIUC, numa_node_id() is faster than cpu_to_node(thiscpu), even if we
> have thiscpu already.  cpu_to_node() is mainly used to get the node of
> NOT current CPU.  So, IMHO, we should only use numa_node_id() in this
> function.
> 

This change?

modified   mm/mempolicy.c
@@ -2502,8 +2502,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
 	pgoff_t ilx;
 	struct zoneref *z;
 	int curnid = folio_nid(folio);
-	int thiscpu = raw_smp_processor_id();
-	int thisnid = cpu_to_node(thiscpu);
+	int thisnid = numa_node_id();
 	int polnid = NUMA_NO_NODE;
 	int ret = NUMA_NO_NODE;
 
@@ -2573,7 +2572,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
 		polnid = thisnid;
 
 		if (!should_numa_migrate_memory(current, folio, curnid,
-						thiscpu))
+						raw_smp_processor_id()))
 			goto out;
 	}
 



-aneesh
  

On 2/20/24 12:02 PM, Aneesh Kumar K.V wrote:
> On 2/20/24 11:55 AM, Huang, Ying wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:
>>
>>> On 2/20/24 6:51 AM, Andrew Morton wrote:
>>>> On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>>>>
>>>>>>> --- a/mm/mempolicy.c
>>>>>>> +++ b/mm/mempolicy.c
>>>>>>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>>>>>>   		if (node_isset(curnid, pol->nodes))
>>>>>>>   			goto out;
>>>>>>>   		z = first_zones_zonelist(
>>>>>>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>>>>>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>>>>>>   				gfp_zone(GFP_HIGHUSER),
>>>>>>>   				&pol->nodes);
>>>>>>>   		polnid = zone_to_nid(z->zone);
>>>>>> 	int thisnid = cpu_to_node(thiscpu);
>>>>>>
>>>>>> Is there any dofference between numa_node_id() and
>>>>>> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
>>>>>> using one here and not the other?
>>>>>
>>>>> Hi Andrew
>>>>>
>>>>> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
>>>>> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.
>>>>
>>>> Sure, but mine was a broader thought: why do we have both?  Is one
>>>> preferable and if so why?
>>>
>>> IIUC these are two helpers to fetch current numa node id. and either of them can be used based on need. The default implementation shows the details.
>>> (One small difference is numa_node_id() can use optimized per cpu reader because it is fetching the per cpu variable of the currently running cpu.)
>>>
>>> #ifndef numa_node_id
>>> /* Returns the number of the current Node. */
>>> static inline int numa_node_id(void)
>>> {
>>> 	return raw_cpu_read(numa_node);
>>> }
>>> #endif
>>>
>>> #ifndef cpu_to_node
>>> static inline int cpu_to_node(int cpu)
>>> {
>>> 	return per_cpu(numa_node, cpu);
>>> }
>>> #endif
>>>
>>> In mpol_misplaced function, we need the cpu details because we are using that in other place (should_numa_migreate_memory()). So it makes it easy
>>> to use cpu_to_node(thiscpu) instead of numa_node_id(). 
>>
>> IIUC, numa_node_id() is faster than cpu_to_node(thiscpu), even if we
>> have thiscpu already.  cpu_to_node() is mainly used to get the node of
>> NOT current CPU.  So, IMHO, we should only use numa_node_id() in this
>> function.
>>
> 
> This change?
> 
> modified   mm/mempolicy.c
> @@ -2502,8 +2502,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>  	pgoff_t ilx;
>  	struct zoneref *z;
>  	int curnid = folio_nid(folio);
> -	int thiscpu = raw_smp_processor_id();
> -	int thisnid = cpu_to_node(thiscpu);
> +	int thisnid = numa_node_id();
>  	int polnid = NUMA_NO_NODE;
>  	int ret = NUMA_NO_NODE;
>  
> @@ -2573,7 +2572,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>  		polnid = thisnid;
>  
>  		if (!should_numa_migrate_memory(current, folio, curnid,
> -						thiscpu))
> +						raw_smp_processor_id()))
>  			goto out;
>  	}
>  
> 

One of the problem with the above change will be the need to make sure smp processor id remaining stable, which
I am not sure we want in this function. With that we can end up with processor id not related to the numa node id
we are using. 

-aneesh
  

"Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:

> On 2/20/24 12:02 PM, Aneesh Kumar K.V wrote:
>> On 2/20/24 11:55 AM, Huang, Ying wrote:
>>> "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:
>>>
>>>> On 2/20/24 6:51 AM, Andrew Morton wrote:
>>>>> On Mon, 19 Feb 2024 14:04:23 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>>>>>
>>>>>>>> --- a/mm/mempolicy.c
>>>>>>>> +++ b/mm/mempolicy.c
>>>>>>>> @@ -2526,7 +2526,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>>>>>>>   		if (node_isset(curnid, pol->nodes))
>>>>>>>>   			goto out;
>>>>>>>>   		z = first_zones_zonelist(
>>>>>>>> -				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>>>>>>> +				node_zonelist(thisnid, GFP_HIGHUSER),
>>>>>>>>   				gfp_zone(GFP_HIGHUSER),
>>>>>>>>   				&pol->nodes);
>>>>>>>>   		polnid = zone_to_nid(z->zone);
>>>>>>> 	int thisnid = cpu_to_node(thiscpu);
>>>>>>>
>>>>>>> Is there any dofference between numa_node_id() and
>>>>>>> cpu_to_node(raw_smp_processor_id())?  And it it explicable that we're
>>>>>>> using one here and not the other?
>>>>>>
>>>>>> Hi Andrew
>>>>>>
>>>>>> Both numa_node_id() and cpu_to_node(raw_smp_processor_id()) return the current execution node id,
>>>>>> Since the current execution node is already fetched at the beginning (thisnid) we can reuse it instead of getting it again.
>>>>>
>>>>> Sure, but mine was a broader thought: why do we have both?  Is one
>>>>> preferable and if so why?
>>>>
>>>> IIUC these are two helpers to fetch current numa node id. and either of them can be used based on need. The default implementation shows the details.
>>>> (One small difference is numa_node_id() can use optimized per cpu reader because it is fetching the per cpu variable of the currently running cpu.)
>>>>
>>>> #ifndef numa_node_id
>>>> /* Returns the number of the current Node. */
>>>> static inline int numa_node_id(void)
>>>> {
>>>> 	return raw_cpu_read(numa_node);
>>>> }
>>>> #endif
>>>>
>>>> #ifndef cpu_to_node
>>>> static inline int cpu_to_node(int cpu)
>>>> {
>>>> 	return per_cpu(numa_node, cpu);
>>>> }
>>>> #endif
>>>>
>>>> In mpol_misplaced function, we need the cpu details because we are using that in other place (should_numa_migreate_memory()). So it makes it easy
>>>> to use cpu_to_node(thiscpu) instead of numa_node_id(). 
>>>
>>> IIUC, numa_node_id() is faster than cpu_to_node(thiscpu), even if we
>>> have thiscpu already.  cpu_to_node() is mainly used to get the node of
>>> NOT current CPU.  So, IMHO, we should only use numa_node_id() in this
>>> function.
>>>
>> 
>> This change?
>> 
>> modified   mm/mempolicy.c
>> @@ -2502,8 +2502,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>  	pgoff_t ilx;
>>  	struct zoneref *z;
>>  	int curnid = folio_nid(folio);
>> -	int thiscpu = raw_smp_processor_id();
>> -	int thisnid = cpu_to_node(thiscpu);
>> +	int thisnid = numa_node_id();
>>  	int polnid = NUMA_NO_NODE;
>>  	int ret = NUMA_NO_NODE;
>>  
>> @@ -2573,7 +2572,7 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
>>  		polnid = thisnid;
>>  
>>  		if (!should_numa_migrate_memory(current, folio, curnid,
>> -						thiscpu))
>> +						raw_smp_processor_id()))
>>  			goto out;
>>  	}
>>  
>> 
>
> One of the problem with the above change will be the need to make sure smp processor id remaining stable, which
> I am not sure we want in this function. With that we can end up with processor id not related to the numa node id
> we are using. 

This isn't an issue now, because mpol_misplaced() are always called with
PTL held.  And, we can still keep thiscpu local variable.

--
Best Regards,
Huang, Ying
  

On Tue 20-02-24 15:22:07, Huang, Ying wrote:
[...]
> This isn't an issue now, because mpol_misplaced() are always called with
> PTL held.  And, we can still keep thiscpu local variable.

yes, this is the case but it would be better if we made that assumption
official by lockdep_assert_held
  

Michal Hocko <mhocko@suse.com> writes:

> On Tue 20-02-24 15:22:07, Huang, Ying wrote:
> [...]
>> This isn't an issue now, because mpol_misplaced() are always called with
>> PTL held.  And, we can still keep thiscpu local variable.
>
> yes, this is the case but it would be better if we made that assumption
> official by lockdep_assert_held
>

How about this folded into this patch?

2 files changed, 12 insertions(+), 4 deletions(-)
mm/memory.c    |  6 ++++--
mm/mempolicy.c | 10 ++++++++--

modified   mm/memory.c
@@ -4879,9 +4879,11 @@ static vm_fault_t do_fault(struct vm_fault *vmf)
 	return ret;
 }
 
-int numa_migrate_prep(struct folio *folio, struct vm_area_struct *vma,
+int numa_migrate_prep(struct folio *folio, struct vm_fault *vmf,
 		      unsigned long addr, int page_nid, int *flags)
 {
+	struct vm_area_struct *vma = vmf->vma;
+
 	folio_get(folio);
 
 	/* Record the current PID acceesing VMA */
@@ -4893,7 +4895,7 @@ int numa_migrate_prep(struct folio *folio, struct vm_area_struct *vma,
 		*flags |= TNF_FAULT_LOCAL;
 	}
 
-	return mpol_misplaced(folio, vma, addr);
+	return mpol_misplaced(folio, vmf, addr);
 }
 
 static vm_fault_t do_numa_page(struct vm_fault *vmf)
modified   mm/mempolicy.c
@@ -2495,18 +2495,24 @@ static inline bool mpol_preferred_should_numa_migrate(int exec_node, int folio_n
  * Return: NUMA_NO_NODE if the page is in a node that is valid for this
  * policy, or a suitable node ID to allocate a replacement folio from.
  */
-int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
+int mpol_misplaced(struct folio *folio, struct vm_fault *vmf,
 		   unsigned long addr)
 {
 	struct mempolicy *pol;
 	pgoff_t ilx;
 	struct zoneref *z;
 	int curnid = folio_nid(folio);
+	struct vm_area_struct *vma = vmf->vma;
 	int thiscpu = raw_smp_processor_id();
-	int thisnid = cpu_to_node(thiscpu);
+	int thisnid = numa_node_id();
 	int polnid = NUMA_NO_NODE;
 	int ret = NUMA_NO_NODE;
 
+	/*
+	 * Make sure ptl is held so that we don't preempt and we
+	 * have a stable smp processor id
+	 */
+	lockdep_assert_held(vmf->ptl);
 	pol = get_vma_policy(vma, addr, folio_order(folio), &ilx);
 	if (!(pol->flags & MPOL_F_MOF))
 		goto out;

[back]
  

Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:

> Michal Hocko <mhocko@suse.com> writes:
>
>> On Tue 20-02-24 15:22:07, Huang, Ying wrote:
>> [...]
>>> This isn't an issue now, because mpol_misplaced() are always called with
>>> PTL held.  And, we can still keep thiscpu local variable.
>>
>> yes, this is the case but it would be better if we made that assumption
>> official by lockdep_assert_held
>>
>
> How about this folded into this patch?
>
> 2 files changed, 12 insertions(+), 4 deletions(-)
> mm/memory.c    |  6 ++++--
> mm/mempolicy.c | 10 ++++++++--
>
> modified   mm/memory.c
> @@ -4879,9 +4879,11 @@ static vm_fault_t do_fault(struct vm_fault *vmf)
>  	return ret;
>  }
>  
> -int numa_migrate_prep(struct folio *folio, struct vm_area_struct *vma,
> +int numa_migrate_prep(struct folio *folio, struct vm_fault *vmf,
>  		      unsigned long addr, int page_nid, int *flags)
>  {
> +	struct vm_area_struct *vma = vmf->vma;
> +
>  	folio_get(folio);
>  
>  	/* Record the current PID acceesing VMA */
> @@ -4893,7 +4895,7 @@ int numa_migrate_prep(struct folio *folio, struct vm_area_struct *vma,
>  		*flags |= TNF_FAULT_LOCAL;
>  	}
>  
> -	return mpol_misplaced(folio, vma, addr);
> +	return mpol_misplaced(folio, vmf, addr);
>  }
>  
>  static vm_fault_t do_numa_page(struct vm_fault *vmf)
> modified   mm/mempolicy.c
> @@ -2495,18 +2495,24 @@ static inline bool mpol_preferred_should_numa_migrate(int exec_node, int folio_n
>   * Return: NUMA_NO_NODE if the page is in a node that is valid for this
>   * policy, or a suitable node ID to allocate a replacement folio from.
>   */
> -int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
> +int mpol_misplaced(struct folio *folio, struct vm_fault *vmf,
>  		   unsigned long addr)
>  {
>  	struct mempolicy *pol;
>  	pgoff_t ilx;
>  	struct zoneref *z;
>  	int curnid = folio_nid(folio);
> +	struct vm_area_struct *vma = vmf->vma;
>  	int thiscpu = raw_smp_processor_id();
> -	int thisnid = cpu_to_node(thiscpu);
> +	int thisnid = numa_node_id();
>  	int polnid = NUMA_NO_NODE;
>  	int ret = NUMA_NO_NODE;
>  
> +	/*
> +	 * Make sure ptl is held so that we don't preempt and we
> +	 * have a stable smp processor id
> +	 */
> +	lockdep_assert_held(vmf->ptl);
>  	pol = get_vma_policy(vma, addr, folio_order(folio), &ilx);
>  	if (!(pol->flags & MPOL_F_MOF))
>  		goto out;
>
> [back]
>  

LGTM, Thanks!

--
Best Regards,
Huang, Ying