> On Jan 12, 2021, at 11:02 AM, Laurent Dufour <ldufour@xxxxxxxxxxxxxxxxxx> wrote: > > Le 12/01/2021 à 17:57, Peter Zijlstra a écrit : >> On Tue, Jan 12, 2021 at 04:47:17PM +0100, Laurent Dufour wrote: >>> Le 12/01/2021 à 12:43, Vinayak Menon a écrit : >>>> Possibility of race against other PTE modifiers >>>> >>>> 1) Fork - We have seen a case of SPF racing with fork marking PTEs RO and that >>>> is described and fixed here https://lore.kernel.org/patchwork/patch/1062672/ >> Right, that's exactly the kind of thing I was worried about. >>>> 2) mprotect - change_protection in mprotect which does the deferred flush is >>>> marked under vm_write_begin/vm_write_end, thus SPF bails out on faults >>>> on those VMAs. >> Sure, mprotect also changes vm_flags, so it really needs that anyway. >>>> 3) userfaultfd - mwriteprotect_range is not protected unlike in (2) above. >>>> But SPF does not take UFFD faults. >>>> 4) hugetlb - hugetlb_change_protection - called from mprotect and covered by >>>> (2) above. >>>> 5) Concurrent faults - SPF does not handle all faults. Only anon page faults. >> What happened to shared/file-backed stuff? ISTR I had that working. > > File-backed mappings are not processed in a speculative way, there were options to manage some of them depending on the underlying file system but that's still not done. > > Shared anonymous mapping, are also not yet handled in a speculative way (vm_ops is not null). > >>>> Of which do_anonymous_page and do_swap_page are NONE/NON-PRESENT->PRESENT >>>> transitions without tlb flush. And I hope do_wp_page with RO->RW is fine as well. >> The tricky one is demotion, specifically write to non-write. >>>> I could not see a case where speculative path cannot see a PTE update done via >>>> a fault on another CPU. >> One you didn't mention is the NUMA balancing scanning crud; although I >> think that's fine, loosing a PTE update there is harmless. But I've not >> thought overly hard on it. > > That's a good point, I need to double check on that side. > >>> You explained it fine. Indeed SPF is handling deferred TLB invalidation by >>> marking the VMA through vm_write_begin/end(), as for the fork case you >>> mentioned. Once the PTL is held, and the VMA's seqcount is checked, the PTE >>> values read are valid. >> That should indeed work, but are we really sure we covered them all? >> Should we invest in better TLBI APIs to make sure we can't get this >> wrong? > > That may be a good option to identify deferred TLB invalidation but I've no clue on what this API would look like. I will send an RFC soon for per-table deferred TLB flushes tracking. The basic idea is to save a generation in the page-struct that tracks when deferred PTE change took place, and track whenever a TLB flush completed. In addition, other users - such as mprotect - would use the tlb_gather interface. Unfortunately, due to limited space in page-struct this would only be possible for 64-bit (and my implementation is only for x86-64). It would still require to do the copying while holding the PTL though.
