On Wed, Dec 7, 2022 at 8:57 PM David Matlack <dmatlack@xxxxxxxxxx> wrote: > > On Tue, Dec 6, 2022 at 12:41 PM James Houghton <jthoughton@xxxxxxxxxx> wrote: > > On Tue, Dec 6, 2022 at 1:01 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > Can you elaborate on what makes it better? Or maybe generate a list of pros and > > > cons? I can think of (dis)advantages for both approaches, but I haven't identified > > > anything that would be a blocking issue for either approach. Doesn't mean there > > > isn't one or more blocking issues, just that I haven't thought of any :-) > > > > Let's see.... so using no-slow-GUP over no UFFD waiting: > > - No need to take mmap_lock in mem fault path. > > - Change the relevant __gfn_to_pfn_memslot callers > > (kvm_faultin_pfn/user_mem_abort/others?) to set `atomic = true` if the > > new CAP is used. > > - No need for a new PF_NO_UFFD_WAIT (would be toggled somewhere > > in/near kvm_faultin_pfn/user_mem_abort). > > - Userspace has to indirectly figure out the state of the page tables > > to know what action to take (which introduces some weirdness, like if > > anyone MADV_DONTNEEDs some guest memory, we need to know). > > I'm no expert but I believe a guest access to MADV_DONTNEED'd GFN > would just cause a new page to be allocated by the kernel. So I think > userspace can still blindly do MADV_POPULATE_WRITE in this case. Were > there any other scenarios you had in mind? MADV_POPULATE_WRITE would drop into handle_userfault() if we're using uffd minor faults after we do MADV_DONTNEED. For uffd minor faults, if the PTE is none (i.e., completely blank, no swap information or anything), then we drop into handle_userfault(). I partially take back what I said. We have to be careful about someone messing with our page tables no matter which API we choose. Here is a better description of the weirdness that we have to put up with given each choice, with this assumption that, normally, we want to UFFDIO_CONTINUE a page exactly once: - For the no-slow-GUP choice, if someone MADV_DONTNEEDed memory and we didn't know about it, we would get stuck in MADV_POPULATE_WRITE. By using UFFD_FEATURE_THREAD_ID, we can tell if we got a userfault for a thread that is in the middle of a MADV_POPULATE_WRITE, and we can try to unblock the thread by doing an extra UFFDIO_CONTINUE. - For the PF_NO_UFFD_WAIT choice, if someone MADV_DONTNEEDed memory, we would just keep trying to start the vCPU without doing anything (we assume some other thread has UFFDIO_CONTINUEd for us). This is basically the same as if we were stuck in MADV_POPULATE_WRITE, and we can try to unblock the thread in a fashion similar to how we would in the other case. So really these approaches have similar requirements for what userspace needs to track. So I think I prefer the no-slow-GUP approach then. > > > - While userfaultfd is registered (so like during post-copy), any > > hva_to_pfn() calls that were resolvable with slow GUP before (without > > dropping into handle_userfault()) will now need to be resolved by > > userspace manually with a call to MADV_POPULATE_WRITE. This extra trip > > to userspace could slow things down. > > Is there any way to enable fast-gup to identify when a PTE is not > present due to userfaultfd specifically without taking the mmap_lock > (e.g. using an unused bit in the PTE)? Then we could avoid extra trips > to userspace for MADV_POPULATE_WRITE. To know if you would have dropped into handle_userfault(), you have to at least check the VMA flags, so at the moment, no. :(
