On 12/04/2017 02:22 PM, Andy Lutomirski wrote:
>> +
>> + this_cpu_write(cpu_tlbstate.invalidate_other, true);
>
> Why do we need this extra variable instead of just looping over all
> other ASIDs and invalidating them? It would be something like:
>
> for (i = 1; i < TLB_NR_DYN_ASIDS; i++) {
> if (i != this_cpu_read(cpu_tlbstate.loaded_mm_asid))
> this_cpu_write(cpu_tlbstate.ctxs[i].ctx_id, 0);
> }
We have loops like this:
for (addr = start; addr < end; addr += PAGE_SIZE)
flush_tlb_single();
Where flush_tlb_single() does a invalidate_pcid_other(). So, inlining
flush_tlb_single() rougly looks like:
for (addr = start; addr < end; addr += PAGE_SIZE) {
invlpg;
for (i = 1; i < TLB_NR_DYN_ASIDS; i++) {
this_cpu_write(cpu_tlbstate.ctxs[i].ctx_id, 0);
}
or, with a "invalidate_other" variable:
for (addr = start; addr < end; addr += PAGE_SIZE) {
invlpg;
this_cpu_write(cpu_tlbstate.ctxs.invalidate_other, 1);
}
The double-for-loop looks a bit wasteful to me.
>> static inline void __flush_tlb_one(unsigned long addr)
>> {
>> count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ONE);
>> __flush_tlb_single(addr);
>> + /*
>> + * Invalidate other address spaces inaccessible to single-page
>> + * invalidation:
>> + */
>
> Ugh. If I'm reading this right, __flush_tlb_single() means "flush one
> user address" and __flush_tlb_one() means "flush one kernel address".
> That's, um, not exactly obvious. Could this be at least commented
> better?
That sounds sane, but let me take a look at it.
Didn't Peter have some patches to do some of that rename?
