On Thu, 20 Sep 2018, John David Anglin wrote: > > In the current state (with the lock), this race condition can't happen. It > > would be: > > > > CPU1 (executing a page fault handler): > > lock pa_tlb_lock > > load *pte from pagetable > > insert pte into TLB > > unlock pa_tlb_lock > > The above two operations might be interchanged when viewed from CPU2. > Is there time for CPU2 to get lock and purge TLB entry before insert? It's hard to say if idtlbt is serializing instruction or not. Maybe it is, but the manual doesn't specify that. Anyway, the current code has "sync" between the idtlbt and unlock, so they can't be interchanged. > The unlock operation will execute after the last access but not be retired > until after insert. > > > > > CPU2 (executing ptep_set_wrprotect) > > spin_lock_irqsave(&pa_tlb_lock, flags); > > set_pte(ptep, pte_wrprotect(*ptep)); > > purge_tlb_entries(mm, addr); > > spin_unlock_irqrestore(&pa_tlb_lock, flags); > > > > --- the purge_tlb_entries function would purge the translation that was > > loaded by CPU1, so there will be no stale entry in the TLB. The lock is > > needed even if the TLB handler doesn't modify the pagetable. > > I understand the issue. I'm just not sure the lock solves the issue on > PA 2.0. Maybe that's why local purge instructions were added in PA > 2.0. If that's the case, then the purge_tlb_entries line needs to be > after the unlock. You are convincing me that using IPI and local purges > may be the only correct solution. If we use IPIs, we can drop the lock from the fault handler. > I'll run some tests with the locks in the TLB handler and routines in > pgtable.h updated. Perhaps disassembling the HP-UX TLB handlers would be the best option :) I suppose they selected the optimal implementation. Mikulas
