Messages in this thread | | | Subject | Re: [patch 51/60] x86/mm: Allow flushing for future ASID switches | From | Dave Hansen <> | Date | Mon, 4 Dec 2017 14:34:53 -0800 |
| |
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?
| |