| Message ID | 20220926115621.13849-1-zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm: use update_mmu_tlb() on the second thread | expand |
On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use > update_mmu_tlb() instead of update_mmu_cache() on the second thread. > > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > Reviewed-by: Muchun Song <songmuchun@bytedance.com> > --- > v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ > > Changelog in v1 -> v2: > - change the subject and commit message (David) > > 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; > } > Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. So it looks good to me, but a confirmation from Bibo Mao might be good.
On 2022/9/26 22:34, David Hildenbrand wrote: > On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use >> update_mmu_tlb() instead of update_mmu_cache() on the second thread. >> >> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >> --- >> v1: >> https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ >> >> Changelog in v1 -> v2: >> - change the subject and commit message (David) >> >> 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; >> } > > > Staring at 7df676974359, it indeed looks like an accidental use [nothing > else in that patch uses update_mmu_cache]. > > So it looks good to me, but a confirmation from Bibo Mao might be good. Thanks, and Hi Bibo, any comments here? :) >
在 2022/9/29 11:07, Qi Zheng 写道: > > > On 2022/9/26 22:34, David Hildenbrand wrote: >> On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use >>> update_mmu_tlb() instead of update_mmu_cache() on the second thread. >>> >>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >>> --- >>> v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ >>> >>> Changelog in v1 -> v2: >>> - change the subject and commit message (David) >>> >>> 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; >>> } >> >> >> Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. >> >> So it looks good to me, but a confirmation from Bibo Mao might be good. > > Thanks, and Hi Bibo, any comments here? :) update_mmu_tlb is defined as empty on loongarch, maybe some lines should be added in file arch/loongarch/include/asm/pgtable.h like this: +#define __HAVE_ARCH_UPDATE_MMU_TLB +#define update_mmu_tlb update_mmu_cache regards bibo mao > >> >
On 2022/9/29 11:27, maobibo wrote: > 在 2022/9/29 11:07, Qi Zheng 写道: >> >> >> On 2022/9/26 22:34, David Hildenbrand wrote: >>> On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use >>>> update_mmu_tlb() instead of update_mmu_cache() on the second thread. >>>> >>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>>> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >>>> --- >>>> v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ >>>> >>>> Changelog in v1 -> v2: >>>> - change the subject and commit message (David) >>>> >>>> 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; >>>> } >>> >>> >>> Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. >>> >>> So it looks good to me, but a confirmation from Bibo Mao might be good. >> >> Thanks, and Hi Bibo, any comments here? :) > > update_mmu_tlb is defined as empty on loongarch, maybe some lines should > be added in file arch/loongarch/include/asm/pgtable.h like this: Seems like a bug? Because there are many other places where update_mmu_tlb() is called. > > +#define __HAVE_ARCH_UPDATE_MMU_TLB > +#define update_mmu_tlb update_mmu_cache If so, I can make the above as a separate fix patch. Thanks, Qi > > regards > bibo mao >> >>> >> >
在 2022/9/29 11:47, Qi Zheng 写道: > > > On 2022/9/29 11:27, maobibo wrote: >> 在 2022/9/29 11:07, Qi Zheng 写道: >>> >>> >>> On 2022/9/26 22:34, David Hildenbrand wrote: >>>> On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use >>>>> update_mmu_tlb() instead of update_mmu_cache() on the second thread. >>>>> >>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>>>> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >>>>> --- >>>>> v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ >>>>> >>>>> Changelog in v1 -> v2: >>>>> - change the subject and commit message (David) >>>>> >>>>> 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; >>>>> } >>>> >>>> >>>> Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. >>>> >>>> So it looks good to me, but a confirmation from Bibo Mao might be good. >>> >>> Thanks, and Hi Bibo, any comments here? :) >> >> update_mmu_tlb is defined as empty on loongarch, maybe some lines should >> be added in file arch/loongarch/include/asm/pgtable.h like this: > > Seems like a bug? Because there are many other places where > update_mmu_tlb() is called. > >> >> +#define __HAVE_ARCH_UPDATE_MMU_TLB >> +#define update_mmu_tlb update_mmu_cache > > If so, I can make the above as a separate fix patch. It sounds good to me. Huacai, do you have any comments? regards bibo, mao > > Thanks, > Qi > >> >> regards >> bibo mao >>> >>>> >>> >> >
Hi, all, > -----原始邮件----- > 发件人: maobibo <maobibo@loongson.cn> > 发送时间:2022-09-29 12:05:33 (星期四) > 收件人: "Qi Zheng" <zhengqi.arch@bytedance.com>, "David Hildenbrand" <david@redhat.com>, akpm@linux-foundation.org, muchun.song@linux.dev, "陈华才" <chenhuacai@loongson.cn> > 抄送: chris@zankel.net, jcmvbkbc@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, "Muchun Song" <songmuchun@bytedance.com> > 主题: Re: [PATCH v2] mm: use update_mmu_tlb() on the second thread > > > > 在 2022/9/29 11:47, Qi Zheng 写道: > > > > > > On 2022/9/29 11:27, maobibo wrote: > >> 在 2022/9/29 11:07, Qi Zheng 写道: > >>> > >>> > >>> On 2022/9/26 22:34, David Hildenbrand wrote: > >>>> On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use > >>>>> update_mmu_tlb() instead of update_mmu_cache() on the second thread. > >>>>> > >>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > >>>>> Reviewed-by: Muchun Song <songmuchun@bytedance.com> > >>>>> --- > >>>>> v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ > >>>>> > >>>>> Changelog in v1 -> v2: > >>>>> - change the subject and commit message (David) > >>>>> > >>>>> 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; > >>>>> } > >>>> > >>>> > >>>> Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. > >>>> > >>>> So it looks good to me, but a confirmation from Bibo Mao might be good. > >>> > >>> Thanks, and Hi Bibo, any comments here? :) > >> > >> update_mmu_tlb is defined as empty on loongarch, maybe some lines should > >> be added in file arch/loongarch/include/asm/pgtable.h like this: > > > > Seems like a bug? Because there are many other places where > > update_mmu_tlb() is called. > > > >> > >> +#define __HAVE_ARCH_UPDATE_MMU_TLB > >> +#define update_mmu_tlb update_mmu_cache > > > > If so, I can make the above as a separate fix patch. > It sounds good to me. > > Huacai, do you have any comments? From my point of view, LoongArch need a fix for this. Huacai > > regards > bibo, mao > > > > Thanks, > > Qi > > > >> > >> regards > >> bibo mao > >>> > >>>> > >>> > >> > > 本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.
On 2022/9/29 16:38, 陈华才 wrote: > Hi, all, > > >> -----原始邮件----- >> 发件人: maobibo <maobibo@loongson.cn> >> 发送时间:2022-09-29 12:05:33 (星期四) >> 收件人: "Qi Zheng" <zhengqi.arch@bytedance.com>, "David Hildenbrand" <david@redhat.com>, akpm@linux-foundation.org, muchun.song@linux.dev, "陈华才" <chenhuacai@loongson.cn> >> 抄送: chris@zankel.net, jcmvbkbc@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, "Muchun Song" <songmuchun@bytedance.com> >> 主题: Re: [PATCH v2] mm: use update_mmu_tlb() on the second thread >> >> >> >> 在 2022/9/29 11:47, Qi Zheng 写道: >>> >>> >>> On 2022/9/29 11:27, maobibo wrote: >>>> 在 2022/9/29 11:07, Qi Zheng 写道: >>>>> >>>>> >>>>> On 2022/9/26 22:34, David Hildenbrand wrote: >>>>>> On 26.09.22 13:56, Qi Zheng 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 in the do_anonymous_page() here, we should use >>>>>>> update_mmu_tlb() instead of update_mmu_cache() on the second thread. >>>>>>> >>>>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>>>>>> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >>>>>>> --- >>>>>>> v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ >>>>>>> >>>>>>> Changelog in v1 -> v2: >>>>>>> - change the subject and commit message (David) >>>>>>> >>>>>>> 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; >>>>>>> } >>>>>> >>>>>> >>>>>> Staring at 7df676974359, it indeed looks like an accidental use [nothing else in that patch uses update_mmu_cache]. >>>>>> >>>>>> So it looks good to me, but a confirmation from Bibo Mao might be good. >>>>> >>>>> Thanks, and Hi Bibo, any comments here? :) >>>> >>>> update_mmu_tlb is defined as empty on loongarch, maybe some lines should >>>> be added in file arch/loongarch/include/asm/pgtable.h like this: >>> >>> Seems like a bug? Because there are many other places where >>> update_mmu_tlb() is called. >>> >>>> >>>> +#define __HAVE_ARCH_UPDATE_MMU_TLB >>>> +#define update_mmu_tlb update_mmu_cache >>> >>> If so, I can make the above as a separate fix patch. >> It sounds good to me. >> >> Huacai, do you have any comments? > From my point of view, LoongArch need a fix for this. OK, will do it in the next version. Thanks. :) > > Huacai >> >> regards >> bibo, mao >>> >>> Thanks, >>> Qi >>> >>>> >>>> regards >>>> bibo mao >>>>> >>>>>> >>>>> >>>> >>> > > > 本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 > This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.
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 in the do_anonymous_page() here, we should use update_mmu_tlb() instead of update_mmu_cache() on the second thread. Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> Reviewed-by: Muchun Song <songmuchun@bytedance.com> --- v1: https://lore.kernel.org/lkml/20220924053239.91661-1-zhengqi.arch@bytedance.com/ Changelog in v1 -> v2: - change the subject and commit message (David) mm/memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)