Re: [v2 PATCH] RISC-V: Optimize tlb flush path.

[Atish Patra]

On Tue, 2019-08-20 at 09:46 +0200, Andreas Schwab wrote:
> On Aug 19 2019, Atish Patra <atish.patra@xxxxxxx> wrote:
> 
> > @@ -42,20 +43,44 @@ static inline void flush_tlb_range(struct
> > vm_area_struct *vma,
> >  
> >  #include <asm/sbi.h>
> >  
> > -static inline void remote_sfence_vma(struct cpumask *cmask,
> > unsigned long start,
> > -				     unsigned long size)
> > +static void __riscv_flush_tlb(struct cpumask *cmask, unsigned long
> > start,
> > +			      unsigned long size)
> 
> cmask can be const.
> 

Sure. Will update it.

> >  {
> >  	struct cpumask hmask;
> > +	unsigned int hartid;
> > +	unsigned int cpuid;
> >  
> >  	cpumask_clear(&hmask);
> > +
> > +	if (!cmask) {
> > +		riscv_cpuid_to_hartid_mask(cpu_online_mask, &hmask);
> > +		goto issue_sfence;
> > +	}
> 
> Wouldn't it make sense to fall through to doing a local flush here?
> 
> 	if (!cmask)
> 		cmask = cpu_online_mask;
> 

cmask NULL is pretty common case and we would  be unnecessarily
executing bunch of instructions everytime while not saving much. Kernel
still have to make an SBI call and OpenSBI is doing a local flush
anyways.

Looking at the code again, I think we can just use cpumask_weight and
do local tlb flush only if local cpu is the only cpu present. 

Otherwise, it will just fall through and call sbi_remote_sfence_vma().

....
....
+
+       cpuid = get_cpu();
+	if (!cmask) {
+               riscv_cpuid_to_hartid_mask(cpu_online_mask, &hmask);
+               goto issue_sfence;
+       }
+
+       
+       if (cpumask_test_cpu(cpuid, cmask) && cpumask_weight(cmask) ==
1) {
+               /* Save trap cost by issuing a local tlb flush here */
+               if ((start == 0 && size == -1) || (size > PAGE_SIZE))
+                       local_flush_tlb_all();
+               else if (size == PAGE_SIZE)
+                       local_flush_tlb_page(start);
+               goto done;
+       }
+
        riscv_cpuid_to_hartid_mask(cmask, &hmask);
+
+issue_sfence:
        sbi_remote_sfence_vma(hmask.bits, start, size);
+done:
+       put_cpu();
 }

This is much simpler than what I had done in v2. I will address the if
condition around size as well.

Regards,
Atish
> Andreas.
>