diff mbox series

[v6,1/4] MIPS: Do not flush tlb page when updating PTE entry

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

Commit Message

Bibo Mao May 25, 2020, 2:52 a.m. UTC
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(+)

Comments

Sergei Shtylyov May 25, 2020, 8:12 a.m. UTC | #1
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
Sergei Shtylyov May 25, 2020, 8:31 a.m. UTC | #2
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
Andrew Morton May 25, 2020, 9:42 p.m. UTC | #3
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.
Bibo Mao May 27, 2020, 1:06 a.m. UTC | #4
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.
Bibo Mao May 27, 2020, 1:07 a.m. UTC | #5
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 mbox series

Patch

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.