| Message ID | 1590375160-6997-1-git-send-email-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v6,1/4] MIPS: Do not flush tlb page when updating PTE entry | expand |
Hello! On 25.05.2020 5:52, Bibo Mao wrote: > It is not necessary to flush tlb page on all CPUs if suitable PTE > entry exists already during page fault handling, just updating > TLB is fine. > > Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. Need empty line here. > V6: > - Add update_mmu_tlb function as empty on all platform except mips > system, we use this function to update local tlb for page fault > smp-race handling > V5: > - define update_mmu_cache function specified on MIPS platform, and > add page fault smp-race stats info > V4: > - add pte_sw_mkyoung function to implement readable privilege, and > this function is only in effect on MIPS system. > - add page valid bit judgement in function pte_modify > V3: > - add detailed changelog, modify typo issue in patch V2 > v2: > - split flush_tlb_fix_spurious_fault and tlb update into two patches > - comments typo modification > - separate tlb update and add pte readable privilege into two patches It was a bad idea to keep the version change log in the 1st patch only, we have either cover letter for that, or all the individual patches... > Signed-off-by: Bibo Mao <maobibo@loongson.cn> [...] MBR, Sergei
On 25.05.2020 11:12, Sergei Shtylyov wrote: >> It is not necessary to flush tlb page on all CPUs if suitable PTE >> entry exists already during page fault handling, just updating >> TLB is fine. >> >> Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. > > Need empty line here. > >> V6: >> - Add update_mmu_tlb function as empty on all platform except mips >> system, we use this function to update local tlb for page fault >> smp-race handling >> V5: >> - define update_mmu_cache function specified on MIPS platform, and >> add page fault smp-race stats info >> V4: >> - add pte_sw_mkyoung function to implement readable privilege, and >> this function is only in effect on MIPS system. >> - add page valid bit judgement in function pte_modify >> V3: >> - add detailed changelog, modify typo issue in patch V2 >> v2: >> - split flush_tlb_fix_spurious_fault and tlb update into two patches >> - comments typo modification >> - separate tlb update and add pte readable privilege into two patches > > It was a bad idea to keep the version change log in the 1st patch only, > we have either cover letter for that, or all the individual patches... Sorry for noticing this only now. With 4 patches, you should have a cover letter anyway... >> Signed-off-by: Bibo Mao <maobibo@loongson.cn> > [...] MBR, Sergei
On Mon, 25 May 2020 10:52:37 +0800 Bibo Mao <maobibo@loongson.cn> wrote: > It is not necessary to flush tlb page on all CPUs if suitable PTE > entry exists already during page fault handling, just updating > TLB is fine. > > Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. > > ... > > --- a/arch/mips/include/asm/pgtable.h > +++ b/arch/mips/include/asm/pgtable.h > @@ -478,6 +478,8 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot) > return __pgprot(prot); > } > > +#define flush_tlb_fix_spurious_fault(vma, address) do { } while (0) > + static inline C would be preferred, if that works. For a number of reasons: - looks nicer - more likely to get a code comment (for some reason) - adds typechecking. So right now a MIPS developer could do struct wibble a; struct wobble b; flush_tlb_fix_spurious_fault(&a, &b); and there would be no compiler warning. Then the code gets merged upstream and in come the embarrassing emails! - avoids unused-var warnings foo() { struct address_space a; struct vm_area_struct v; flush_tlb_fix_spurious_fault(&v, &a); } will generate unused-variable warnings if flush_tlb_fix_spurious_fault() is a macro. Making flush_tlb_fix_spurious_fault() inlined C prevents this.
On 05/26/2020 05:42 AM, Andrew Morton wrote: > On Mon, 25 May 2020 10:52:37 +0800 Bibo Mao <maobibo@loongson.cn> wrote: > >> It is not necessary to flush tlb page on all CPUs if suitable PTE >> entry exists already during page fault handling, just updating >> TLB is fine. >> >> Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. >> >> ... >> >> --- a/arch/mips/include/asm/pgtable.h >> +++ b/arch/mips/include/asm/pgtable.h >> @@ -478,6 +478,8 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot) >> return __pgprot(prot); >> } >> >> +#define flush_tlb_fix_spurious_fault(vma, address) do { } while (0) >> + > > static inline C would be preferred, if that works. For a number of reasons: > > - looks nicer > > - more likely to get a code comment (for some reason) > > - adds typechecking. So right now a MIPS developer could do > > struct wibble a; > struct wobble b; > > flush_tlb_fix_spurious_fault(&a, &b); > > and there would be no compiler warning. Then the code gets merged > upstream and in come the embarrassing emails! > > - avoids unused-var warnings > > foo() > { > struct address_space a; > struct vm_area_struct v; > > flush_tlb_fix_spurious_fault(&v, &a); > } > > will generate unused-variable warnings if > flush_tlb_fix_spurious_fault() is a macro. Making > flush_tlb_fix_spurious_fault() inlined C prevents this. > Sure, I will modify this and send another version.
On 05/25/2020 04:31 PM, Sergei Shtylyov wrote: > On 25.05.2020 11:12, Sergei Shtylyov wrote: > >>> It is not necessary to flush tlb page on all CPUs if suitable PTE >>> entry exists already during page fault handling, just updating >>> TLB is fine. >>> >>> Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. >> >> Need empty line here. >> >>> V6: >>> - Add update_mmu_tlb function as empty on all platform except mips >>> system, we use this function to update local tlb for page fault >>> smp-race handling >>> V5: >>> - define update_mmu_cache function specified on MIPS platform, and >>> add page fault smp-race stats info >>> V4: >>> - add pte_sw_mkyoung function to implement readable privilege, and >>> this function is only in effect on MIPS system. >>> - add page valid bit judgement in function pte_modify >>> V3: >>> - add detailed changelog, modify typo issue in patch V2 >>> v2: >>> - split flush_tlb_fix_spurious_fault and tlb update into two patches >>> - comments typo modification >>> - separate tlb update and add pte readable privilege into two patches >> >> It was a bad idea to keep the version change log in the 1st patch only, >> we have either cover letter for that, or all the individual patches... > > Sorry for noticing this only now. With 4 patches, you should have a cover letter anyway... Thanks for reviewing my patch, a cover letter will be added. > >>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >> [...] > > MBR, Sergei
diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h index 9b01d2d..0d625c2 100644 --- a/arch/mips/include/asm/pgtable.h +++ b/arch/mips/include/asm/pgtable.h @@ -478,6 +478,8 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot) return __pgprot(prot); } +#define flush_tlb_fix_spurious_fault(vma, address) do { } while (0) + /* * Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to.
It is not necessary to flush tlb page on all CPUs if suitable PTE entry exists already during page fault handling, just updating TLB is fine. Here redefine flush_tlb_fix_spurious_fault as empty on MIPS system. V6: - Add update_mmu_tlb function as empty on all platform except mips system, we use this function to update local tlb for page fault smp-race handling V5: - define update_mmu_cache function specified on MIPS platform, and add page fault smp-race stats info V4: - add pte_sw_mkyoung function to implement readable privilege, and this function is only in effect on MIPS system. - add page valid bit judgement in function pte_modify V3: - add detailed changelog, modify typo issue in patch V2 v2: - split flush_tlb_fix_spurious_fault and tlb update into two patches - comments typo modification - separate tlb update and add pte readable privilege into two patches Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- arch/mips/include/asm/pgtable.h | 2 ++ 1 file changed, 2 insertions(+)