Re: [PATCH 1/2] mm/numa: no task_numa_fault() call if page table is changed

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2024/8/8 22:21, Zi Yan wrote:
> On 8 Aug 2024, at 10:14, David Hildenbrand wrote:
> 
>> On 08.08.24 16:13, Zi Yan wrote:
>>> On 8 Aug 2024, at 4:22, David Hildenbrand wrote:
>>>
>>>> On 08.08.24 05:19, Baolin Wang wrote:
>>>>>
>>>>>
...
>>>> Agreed, maybe we should simply handle that right away and replace the "goto out;" users by "return 0;".
>>>>
>>>> Then, just copy the 3 LOC.
>>>>
>>>> For mm/memory.c that would be:
>>>>
>>>> diff --git a/mm/memory.c b/mm/memory.c
>>>> index 67496dc5064f..410ba50ca746 100644
>>>> --- a/mm/memory.c
>>>> +++ b/mm/memory.c
>>>> @@ -5461,7 +5461,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
>>>>            if (unlikely(!pte_same(old_pte, vmf->orig_pte))) {
>>>>                   pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>> -               goto out;
>>>> +               return 0;
>>>>           }
>>>>            pte = pte_modify(old_pte, vma->vm_page_prot);
>>>> @@ -5528,15 +5528,14 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
>>>>                   vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>>>                                                  vmf->address, &vmf->ptl);
>>>>                   if (unlikely(!vmf->pte))
>>>> -                       goto out;
>>>> +                       return 0;
>>>>                   if (unlikely(!pte_same(ptep_get(vmf->pte), vmf->orig_pte))) {
>>>>                           pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>> -                       goto out;
>>>> +                       return 0;
>>>>                   }
>>>>                   goto out_map;
>>>>           }
>>>>    -out:
>>>>           if (nid != NUMA_NO_NODE)
>>>>                   task_numa_fault(last_cpupid, nid, nr_pages, flags);
>>>>           return 0;

Maybe drop this part too,

diff --git a/mm/memory.c b/mm/memory.c
index 410ba50ca746..07343c1469e0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5523,6 +5523,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
         if (!migrate_misplaced_folio(folio, vma, target_nid)) {
                 nid = target_nid;
                 flags |= TNF_MIGRATED;
+               goto out;
         } else {
                 flags |= TNF_MIGRATE_FAIL;
                 vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
@@ -5533,12 +5534,8 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
                         pte_unmap_unlock(vmf->pte, vmf->ptl);
                         return 0;
                 }
-               goto out_map;
         }

-       if (nid != NUMA_NO_NODE)
-               task_numa_fault(last_cpupid, nid, nr_pages, flags);
-       return 0;
  out_map:
         /*
          * Make it present again, depending on how arch implements
@@ -5551,6 +5548,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
                 numa_rebuild_single_mapping(vmf, vma, vmf->address, 
vmf->pte,
                                             writable);
         pte_unmap_unlock(vmf->pte, vmf->ptl);
+out:
         if (nid != NUMA_NO_NODE)
                 task_numa_fault(last_cpupid, nid, nr_pages, flags);
         return 0;


>>>> @@ -5552,7 +5551,9 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
>>>>                   numa_rebuild_single_mapping(vmf, vma, vmf->address, vmf->pte,
>>>>                                               writable);
>>>>           pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>> -       goto out;
>>>> +       if (nid != NUMA_NO_NODE)
>>>> +               task_numa_fault(last_cpupid, nid, nr_pages, flags);
>>>> +       return 0;
>>>>    }
>>>
>>> Looks good to me. Thanks.
>>>
>>> Hi Andrew,
>>>
>>> Should I resend this for an easy back porting? Or you want to fold David’s
>>> changes in directly?
>>
>> Note that I didn't touch huge_memory.c. So maybe just send a fixup on top?
> 
> Got it. The fixup is attached.
> 
> Best Regards,
> Yan, Zi