* [PATCH] MIPS: Do not flush tlb when setting pmd entry
@ 2020-06-03 9:42 Bibo Mao
2020-06-15 10:14 ` Thomas Bogendoerfer
0 siblings, 1 reply; 7+ messages in thread
From: Bibo Mao @ 2020-06-03 9:42 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, linux-kernel
Function set_pmd_at is to set pmd entry, if tlb entry need to
be flushed, there exists pmdp_huge_clear_flush alike function
before set_pmd_at is called. So it is not necessary to call
flush_tlb_all in this function.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/mips/mm/pgtable-64.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/mips/mm/pgtable-64.c b/arch/mips/mm/pgtable-64.c
index 6fd6e96..a236752 100644
--- a/arch/mips/mm/pgtable-64.c
+++ b/arch/mips/mm/pgtable-64.c
@@ -101,7 +101,6 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
pmd_t *pmdp, pmd_t pmd)
{
*pmdp = pmd;
- flush_tlb_all();
}
void __init pagetable_init(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-03 9:42 [PATCH] MIPS: Do not flush tlb when setting pmd entry Bibo Mao
@ 2020-06-15 10:14 ` Thomas Bogendoerfer
2020-06-16 10:34 ` maobibo
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Bogendoerfer @ 2020-06-15 10:14 UTC (permalink / raw)
To: Bibo Mao; +Cc: linux-mips, linux-kernel
On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
> Function set_pmd_at is to set pmd entry, if tlb entry need to
> be flushed, there exists pmdp_huge_clear_flush alike function
> before set_pmd_at is called. So it is not necessary to call
> flush_tlb_all in this function.
have you checked all set_pmd_at() calls ? I found a few case where
it's not clear to me, if tlb flushing is done... If you think this
is still the right thing to do, please change arch/mips/mm/pgtable-32.c
as well.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-15 10:14 ` Thomas Bogendoerfer
@ 2020-06-16 10:34 ` maobibo
2020-06-17 11:14 ` Thomas Bogendoerfer
0 siblings, 1 reply; 7+ messages in thread
From: maobibo @ 2020-06-16 10:34 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, linux-kernel
On 06/15/2020 06:14 PM, Thomas Bogendoerfer wrote:
> On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
>> Function set_pmd_at is to set pmd entry, if tlb entry need to
>> be flushed, there exists pmdp_huge_clear_flush alike function
>> before set_pmd_at is called. So it is not necessary to call
>> flush_tlb_all in this function.
>
> have you checked all set_pmd_at() calls ? I found a few case where
> it's not clear to me, if tlb flushing is done... If you think this
> is still the right thing to do, please change arch/mips/mm/pgtable-32.c
> as well.
well, I will double check this and do more testing about thp and hugepage.
regards
bibo,mao
>
> Thomas.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-16 10:34 ` maobibo
@ 2020-06-17 11:14 ` Thomas Bogendoerfer
2020-06-20 3:47 ` maobibo
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Bogendoerfer @ 2020-06-17 11:14 UTC (permalink / raw)
To: maobibo; +Cc: linux-mips, linux-kernel
On Tue, Jun 16, 2020 at 06:34:21PM +0800, maobibo wrote:
>
>
> On 06/15/2020 06:14 PM, Thomas Bogendoerfer wrote:
> > On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
> >> Function set_pmd_at is to set pmd entry, if tlb entry need to
> >> be flushed, there exists pmdp_huge_clear_flush alike function
> >> before set_pmd_at is called. So it is not necessary to call
> >> flush_tlb_all in this function.
> >
> > have you checked all set_pmd_at() calls ? I found a few case where
> > it's not clear to me, if tlb flushing is done... If you think this
> > is still the right thing to do, please change arch/mips/mm/pgtable-32.c
> > as well.
> well, I will double check this and do more testing about thp and hugepage.
I was more concerned about
fs/dax.c
fs/proc/task_mmu.c
mm/rmap.c
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-17 11:14 ` Thomas Bogendoerfer
@ 2020-06-20 3:47 ` maobibo
2020-06-22 15:48 ` Thomas Bogendoerfer
0 siblings, 1 reply; 7+ messages in thread
From: maobibo @ 2020-06-20 3:47 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, linux-kernel
On 06/17/2020 07:14 PM, Thomas Bogendoerfer wrote:
> On Tue, Jun 16, 2020 at 06:34:21PM +0800, maobibo wrote:
>>
>>
>> On 06/15/2020 06:14 PM, Thomas Bogendoerfer wrote:
>>> On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
>>>> Function set_pmd_at is to set pmd entry, if tlb entry need to
>>>> be flushed, there exists pmdp_huge_clear_flush alike function
>>>> before set_pmd_at is called. So it is not necessary to call
>>>> flush_tlb_all in this function.
>>>
>>> have you checked all set_pmd_at() calls ? I found a few case where
>>> it's not clear to me, if tlb flushing is done... If you think this
>>> is still the right thing to do, please change arch/mips/mm/pgtable-32.c
>>> as well.
>> well, I will double check this and do more testing about thp and hugepage.
>
> I was more concerned about
>
> fs/dax.c
> fs/proc/task_mmu.c
> mm/rmap.c
I think that flush_tlb_all should not be called in function set_pmd_at
on mips platform. However update_mmu_cache_pmd() should be called __after__
set_pmd_at() function to update tlb entry at some places, it is another issue.
Here is my analysis in the three files where set_pmd_at is called.
in file fs/dax.c
------------------------------------------------------
if (pmdp) {
#ifdef CONFIG_FS_DAX_PMD
pmd_t pmd;
if (pfn != pmd_pfn(*pmdp))
goto unlock_pmd;
if (!pmd_dirty(*pmdp) && !pmd_write(*pmdp))
goto unlock_pmd;
flush_cache_page(vma, address, pfn);
pmd = pmdp_invalidate(vma, address, pmdp);
pmd = pmd_wrprotect(pmd);
pmd = pmd_mkclean(pmd);
set_pmd_at(vma->vm_mm, address, pmdp, pmd);
unlock_pmd:
#endif
------------------------------------------------------
pmdp_invalidate is called here to flush pmd range already, it is not necessary
to flush pmd range in function set_pmd_at
------------------------------------------------------
if (!pmd_none(*(vmf->pmd))) {
spin_unlock(ptl);
goto fallback;
}
if (pgtable) {
pgtable_trans_huge_deposit(vma->vm_mm, vmf->pmd, pgtable);
mm_inc_nr_ptes(vma->vm_mm);
}
pmd_entry = mk_pmd(zero_page, vmf->vma->vm_page_prot);
pmd_entry = pmd_mkhuge(pmd_entry);
set_pmd_at(vmf->vma->vm_mm, pmd_addr, vmf->pmd, pmd_entry);
spin_unlock(ptl);
trace_dax_pmd_load_hole(inode, vmf, zero_page, *entry);
return VM_FAULT_NOPAGE;
------------------------------------------------------
pmd entry is none, does not need to flush pmd range
in file fs/proc/task_mmu.c
------------------------------------------------------
static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
unsigned long addr, pmd_t *pmdp)
{
pmd_t old, pmd = *pmdp;
if (pmd_present(pmd)) {
/* See comment in change_huge_pmd() */
old = pmdp_invalidate(vma, addr, pmdp);
if (pmd_dirty(old))
pmd = pmd_mkdirty(pmd);
if (pmd_young(old))
pmd = pmd_mkyoung(pmd);
pmd = pmd_wrprotect(pmd);
pmd = pmd_clear_soft_dirty(pmd);
set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
} else if (is_migration_entry(pmd_to_swp_entry(pmd))) {
pmd = pmd_swp_clear_soft_dirty(pmd);
set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
}
}
------------------------------------------------------
At the first place where set_pmd_at is called, pmdp_invalidate is called to
flush pmd range. At the second place, on mips system pmd_swp_clear_soft_dirty(pmd)
is equal to pmd, pmd entry has no change, does not need to flush pmd range
in file linux/mm/rmap.c:
------------------------------------------------------
pmd_t *pmd = pvmw.pmd;
pmd_t entry;
if (!pmd_dirty(*pmd) && !pmd_write(*pmd))
continue;
flush_cache_page(vma, address, page_to_pfn(page));
entry = pmdp_invalidate(vma, address, pmd);
entry = pmd_wrprotect(entry);
entry = pmd_mkclean(entry);
set_pmd_at(vma->vm_mm, address, pmd, entry);
ret = 1;
------------------------------------------------------
pmdp_invalidate is called to flush pmd range, does not need to flush pmd range
again in function set_pmd_at.
>
> Thomas.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-20 3:47 ` maobibo
@ 2020-06-22 15:48 ` Thomas Bogendoerfer
2020-06-23 6:27 ` maobibo
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Bogendoerfer @ 2020-06-22 15:48 UTC (permalink / raw)
To: maobibo; +Cc: linux-mips, linux-kernel
On Sat, Jun 20, 2020 at 11:47:35AM +0800, maobibo wrote:
>
>
> On 06/17/2020 07:14 PM, Thomas Bogendoerfer wrote:
> > On Tue, Jun 16, 2020 at 06:34:21PM +0800, maobibo wrote:
> >>
> >>
> >> On 06/15/2020 06:14 PM, Thomas Bogendoerfer wrote:
> >>> On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
> >>>> Function set_pmd_at is to set pmd entry, if tlb entry need to
> >>>> be flushed, there exists pmdp_huge_clear_flush alike function
> >>>> before set_pmd_at is called. So it is not necessary to call
> >>>> flush_tlb_all in this function.
> >>>
> >>> have you checked all set_pmd_at() calls ? I found a few case where
> >>> it's not clear to me, if tlb flushing is done... If you think this
> >>> is still the right thing to do, please change arch/mips/mm/pgtable-32.c
> >>> as well.
> >> well, I will double check this and do more testing about thp and hugepage.
> >
> > I was more concerned about
> >
> > fs/dax.c
> > fs/proc/task_mmu.c
> > mm/rmap.c
>
> I think that flush_tlb_all should not be called in function set_pmd_at
> on mips platform. However update_mmu_cache_pmd() should be called __after__
> set_pmd_at() function to update tlb entry at some places, it is another issue.
> Here is my analysis in the three files where set_pmd_at is called.
> [..]
thank you for confirming that we are good with removing flush_tlb_all().
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MIPS: Do not flush tlb when setting pmd entry
2020-06-22 15:48 ` Thomas Bogendoerfer
@ 2020-06-23 6:27 ` maobibo
0 siblings, 0 replies; 7+ messages in thread
From: maobibo @ 2020-06-23 6:27 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, linux-kernel
On 06/22/2020 11:48 PM, Thomas Bogendoerfer wrote:
> On Sat, Jun 20, 2020 at 11:47:35AM +0800, maobibo wrote:
>>
>>
>> On 06/17/2020 07:14 PM, Thomas Bogendoerfer wrote:
>>> On Tue, Jun 16, 2020 at 06:34:21PM +0800, maobibo wrote:
>>>>
>>>>
>>>> On 06/15/2020 06:14 PM, Thomas Bogendoerfer wrote:
>>>>> On Wed, Jun 03, 2020 at 05:42:13PM +0800, Bibo Mao wrote:
>>>>>> Function set_pmd_at is to set pmd entry, if tlb entry need to
>>>>>> be flushed, there exists pmdp_huge_clear_flush alike function
>>>>>> before set_pmd_at is called. So it is not necessary to call
>>>>>> flush_tlb_all in this function.
>>>>>
>>>>> have you checked all set_pmd_at() calls ? I found a few case where
>>>>> it's not clear to me, if tlb flushing is done... If you think this
>>>>> is still the right thing to do, please change arch/mips/mm/pgtable-32.c
>>>>> as well.
>>>> well, I will double check this and do more testing about thp and hugepage.
>>>
>>> I was more concerned about
>>>
>>> fs/dax.c
>>> fs/proc/task_mmu.c
>>> mm/rmap.c
>>
>> I think that flush_tlb_all should not be called in function set_pmd_at
>> on mips platform. However update_mmu_cache_pmd() should be called __after__
>> set_pmd_at() function to update tlb entry at some places, it is another issue.
>> Here is my analysis in the three files where set_pmd_at is called.
>> [..]
>
> thank you for confirming that we are good with removing flush_tlb_all().
Sorry, there is something wrong if remove flush_tlb_all(). If pmd_none is true,
pmd points to invalid_pte_table, maybe there exists pte entry with normal page size
for fault address. And we need invalidate this pte entry like it is done in function build_huge_handler_tail in file arch/mips/mm/tlbex.c
I will send another patch.
>
> Thomas.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-23 6:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 9:42 [PATCH] MIPS: Do not flush tlb when setting pmd entry Bibo Mao
2020-06-15 10:14 ` Thomas Bogendoerfer
2020-06-16 10:34 ` maobibo
2020-06-17 11:14 ` Thomas Bogendoerfer
2020-06-20 3:47 ` maobibo
2020-06-22 15:48 ` Thomas Bogendoerfer
2020-06-23 6:27 ` maobibo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).