| Message ID | 20220924053239.91661-1-zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: fix misuse of update_mmu_cache() in do_anonymous_page() | expand |
> On Sep 24, 2022, at 13:32, Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> 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 <songmuchun@bytedance.com>
On 25.09.22 03:43, Muchun Song wrote: > > >> On Sep 24, 2022, at 13:32, Qi Zheng <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> > > 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?
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 <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> >> >> 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? >
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 <zhengqi.arch@bytedance.com> 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 <zhengqi.arch@bytedance.com> >>> >>> 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 :)
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 <zhengqi.arch@bytedance.com> >>>>> 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 <zhengqi.arch@bytedance.com> >>>> >>>> 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. :) >
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; }
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 <zhengqi.arch@bytedance.com> --- mm/memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)