Re: [PATCH 1/2] mm/numa: no task_numa_fault() call if page table is changed
From: Zi Yan
Date: Thu Aug 08 2024 - 11:13:57 EST
On 8 Aug 2024, at 10:57, Zi Yan wrote:
> On 8 Aug 2024, at 10:36, Kefeng Wang wrote:
>
>> 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;
>
> Even better. Thanks. The updated fixup is attached.
Update the fixup, if Andrew wants to fold it in instead of a resend, again to fix a typo causing compilation failure.
Best Regards,
Yan, Zi
From b42f0e90ed0b4117139cf66de2d6f83e3d8bcf8d Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@xxxxxxxxxx>
Date: Thu, 8 Aug 2024 10:18:42 -0400
Subject: [PATCH] fixup! mm/numa: no task_numa_fault() call if page table is
changed
---
mm/huge_memory.c | 18 +++++++-----------
mm/memory.c | 19 ++++++++-----------
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index a3c018f2b554..4e4364a17e6d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1681,7 +1681,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf)
vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
if (unlikely(!pmd_same(oldpmd, *vmf->pmd))) {
spin_unlock(vmf->ptl);
- goto out;
+ return 0;
}
pmd = pmd_modify(oldpmd, vma->vm_page_prot);
@@ -1724,23 +1724,16 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf)
if (!migrate_misplaced_folio(folio, vma, target_nid)) {
flags |= TNF_MIGRATED;
nid = target_nid;
+ goto out;
} else {
flags |= TNF_MIGRATE_FAIL;
vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
if (unlikely(!pmd_same(oldpmd, *vmf->pmd))) {
spin_unlock(vmf->ptl);
- goto out;
+ return 0;
}
- goto out_map;
}
-count_fault:
- if (nid != NUMA_NO_NODE)
- task_numa_fault(last_cpupid, nid, HPAGE_PMD_NR, flags);
-
-out:
- return 0;
-
out_map:
/* Restore the PMD */
pmd = pmd_modify(oldpmd, vma->vm_page_prot);
@@ -1750,7 +1743,10 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf)
set_pmd_at(vma->vm_mm, haddr, vmf->pmd, pmd);
update_mmu_cache_pmd(vma, vmf->address, vmf->pmd);
spin_unlock(vmf->ptl);
- goto count_fault;
+out:
+ if (nid != NUMA_NO_NODE)
+ task_numa_fault(last_cpupid, nid, HPAGE_PMD_NR, flags);
+ return 0;
}
/*
diff --git a/mm/memory.c b/mm/memory.c
index 503d493263df..d9b1dff9dc57 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);
@@ -5523,24 +5523,18 @@ 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,
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;
}
-
-count_fault:
- if (nid != NUMA_NO_NODE)
- task_numa_fault(last_cpupid, nid, nr_pages, flags);
-out:
- return 0;
out_map:
/*
* Make it present again, depending on how arch implements
@@ -5553,7 +5547,10 @@ 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 count_fault;
+out:
+ if (nid != NUMA_NO_NODE)
+ task_numa_fault(last_cpupid, nid, nr_pages, flags);
+ return 0;
}
static inline vm_fault_t create_huge_pmd(struct vm_fault *vmf)
--
2.43.0
Attachment:
signature.asc
Description: OpenPGP digital signature