2022-09-24 05:51:48

by Qi Zheng

[permalink] [raw]
Subject: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()

As message in commit 7df676974359 ("mm/memory.c: Update local TLB
if PTE entry exists") said, we should update local TLB only on the
second thread. So fix the misuse of update_mmu_cache() by using
update_mmu_tlb() in the do_anonymous_page().

Signed-off-by: Qi Zheng <[email protected]>
---
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 118e5f023597..9e11c783ba0e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4122,7 +4122,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
&vmf->ptl);
if (!pte_none(*vmf->pte)) {
- update_mmu_cache(vma, vmf->address, vmf->pte);
+ update_mmu_tlb(vma, vmf->address, vmf->pte);
goto release;
}

--
2.20.1


2022-09-25 01:54:46

by Muchun Song

[permalink] [raw]
Subject: Re: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()



> On Sep 24, 2022, at 13:32, Qi Zheng <[email protected]> wrote:
>
> As message in commit 7df676974359 ("mm/memory.c: Update local TLB
> if PTE entry exists") said, we should update local TLB only on the
> second thread. So fix the misuse of update_mmu_cache() by using
> update_mmu_tlb() in the do_anonymous_page().
>
> Signed-off-by: Qi Zheng <[email protected]>

The change looks good to me. However, I am not sure what is the user-visible
effect to xtensa users. So Cc xtensa’s maintainer and the author of 7df676974359
to double check this.


But anyway:

Reviewed-by: Muchun Song <[email protected]>


2022-09-26 09:16:09

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()

On 26.09.22 10:41, Qi Zheng wrote:
>
>
> On 2022/9/26 16:32, David Hildenbrand wrote:
>> On 25.09.22 03:43, Muchun Song wrote:
>>>
>>>
>>>> On Sep 24, 2022, at 13:32, Qi Zheng <[email protected]> wrote:
>>>>
>>>> As message in commit 7df676974359 ("mm/memory.c: Update local TLB
>>>> if PTE entry exists") said, we should update local TLB only on the
>>>> second thread. So fix the misuse of update_mmu_cache() by using
>>>> update_mmu_tlb() in the do_anonymous_page().
>>>>
>>>> Signed-off-by: Qi Zheng <[email protected]>
>>>
>>> The change looks good to me. However, I am not sure what is the
>>> user-visible
>>> effect to xtensa users. So Cc xtensa’s maintainer and the author of
>>> 7df676974359
>>> to double check this.
>>
>> And if there is one, do we have a fixes tag?
>
> IIUC, there's only a performance difference here, so maybe there's no
> need to add the fixes tag?

Maybe be careful with the usage of "fix" in subject/description then and
point that out in the description :)

--
Thanks,

David / dhildenb

2022-09-26 09:16:30

by Qi Zheng

[permalink] [raw]
Subject: Re: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()



On 2022/9/26 16:32, David Hildenbrand wrote:
> On 25.09.22 03:43, Muchun Song wrote:
>>
>>
>>> On Sep 24, 2022, at 13:32, Qi Zheng <[email protected]> wrote:
>>>
>>> As message in commit 7df676974359 ("mm/memory.c: Update local TLB
>>> if PTE entry exists") said, we should update local TLB only on the
>>> second thread. So fix the misuse of update_mmu_cache() by using
>>> update_mmu_tlb() in the do_anonymous_page().
>>>
>>> Signed-off-by: Qi Zheng <[email protected]>
>>
>> The change looks good to me. However, I am not sure what is the
>> user-visible
>> effect to xtensa users. So Cc xtensa’s maintainer and the author of
>> 7df676974359
>> to double check this.
>
> And if there is one, do we have a fixes tag?

IIUC, there's only a performance difference here, so maybe there's no
need to add the fixes tag?

>

--
Thanks,
Qi

2022-09-26 09:16:35

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()

On 25.09.22 03:43, Muchun Song wrote:
>
>
>> On Sep 24, 2022, at 13:32, Qi Zheng <[email protected]> wrote:
>>
>> As message in commit 7df676974359 ("mm/memory.c: Update local TLB
>> if PTE entry exists") said, we should update local TLB only on the
>> second thread. So fix the misuse of update_mmu_cache() by using
>> update_mmu_tlb() in the do_anonymous_page().
>>
>> Signed-off-by: Qi Zheng <[email protected]>
>
> The change looks good to me. However, I am not sure what is the user-visible
> effect to xtensa users. So Cc xtensa’s maintainer and the author of 7df676974359
> to double check this.

And if there is one, do we have a fixes tag?

--
Thanks,

David / dhildenb

2022-09-26 09:17:03

by Qi Zheng

[permalink] [raw]
Subject: Re: [PATCH] mm: fix misuse of update_mmu_cache() in do_anonymous_page()



On 2022/9/26 16:42, David Hildenbrand wrote:
> On 26.09.22 10:41, Qi Zheng wrote:
>>
>>
>> On 2022/9/26 16:32, David Hildenbrand wrote:
>>> On 25.09.22 03:43, Muchun Song wrote:
>>>>
>>>>
>>>>> On Sep 24, 2022, at 13:32, Qi Zheng <[email protected]>
>>>>> wrote:
>>>>>
>>>>> As message in commit 7df676974359 ("mm/memory.c: Update local TLB
>>>>> if PTE entry exists") said, we should update local TLB only on the
>>>>> second thread. So fix the misuse of update_mmu_cache() by using
>>>>> update_mmu_tlb() in the do_anonymous_page().
>>>>>
>>>>> Signed-off-by: Qi Zheng <[email protected]>
>>>>
>>>> The change looks good to me. However, I am not sure what is the
>>>> user-visible
>>>> effect to xtensa users. So Cc xtensa’s maintainer and the author of
>>>> 7df676974359
>>>> to double check this.
>>>
>>> And if there is one, do we have a fixes tag?
>>
>> IIUC, there's only a performance difference here, so maybe there's no
>> need to add the fixes tag?
>
> Maybe be careful with the usage of "fix" in subject/description then and
> point that out in the description :)

Ah, will change subject/description in the v2, thanks. :)

>

--
Thanks,
Qi