Dave Hansen <[email protected]> wrote: > On 06/14/2016 09:47 AM, Nadav Amit wrote: >> Lukasz Anaczkowski <[email protected]> wrote: >> >>>> From: Andi Kleen <[email protected]> >>>> +void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep) >>>> +{ >> Here there should be a call to smp_mb__after_atomic() to synchronize with >> switch_mm. I submitted a similar patch, which is still pending (hint). >> >>>> + if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids) { >>>> + trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL); >>>> + flush_tlb_others(mm_cpumask(mm), mm, addr, >>>> + addr + PAGE_SIZE); >>>> + mb(); >>>> + set_pte(ptep, __pte(0)); >>>> + } >>>> +} > > Shouldn't that barrier be incorporated in the TLB flush code itself and > not every single caller (like this code is)? > > It is insane to require individual TLB flushers to be concerned with the > barriers.
IMHO it is best to use existing flushing interfaces instead of creating new ones. In theory, fix_pte_leak could have used flush_tlb_page. But the problem is that flush_tlb_page requires the vm_area_struct as an argument, which ptep_get_and_clear (and others) do not have. I don’t know which architecture needs the vm_area_struct, since x86 and some others I looked at (e.g., ARM) only need the mm_struct. Nadav
