Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> writes:
> 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
IMHO, migration success is normal path, while migration failure is error
processing path. If so, it's better to use "goto" for error processing
instead of normal path.
> @@ -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;
>
>
--
Best Regards,
Huang, Ying
