lkml.org 
[lkml]   [2017]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [patch 51/60] x86/mm: Allow flushing for future ASID switches
From
Date
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?

\
 
 \ /
  Last update: 2017-12-04 23:36    [W:0.059 / U:8.544 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site