On Tue, Dec 22, 2020 at 08:36:04PM -0700, Yu Zhao wrote: > In your patch, do we need to take wrprotect_rwsem in > handle_userfault() as well? Otherwise, it seems userspace would have > to synchronize between its wrprotect ioctl and fault handler? i.e., > the fault hander needs to be aware that the content of write- > protected pages can actually change before the iotcl returns.
The handle_userfault() thread should be sleeping until another uffd_wp_resolve fixes the page fault for it. However when the uffd_wp_resolve ioctl comes, then rwsem (either the group rwsem lock as Andrea proposed, or the mmap_sem, or any new rwsem lock we'd like to introduce, maybe per-uffd rather than per-mm) should have guaranteed the previous wr-protect ioctls are finished and tlb must have been flushed until this thread continues. And I don't know why it matters even if the data changed - IMHO what uffd-wp wants to do is simply to make sure after wr-protect ioctl returns to userspace, no change on the page should ever happen anymore. So "whether data changed" seems matter more on the ioctl thread rather than the handle_userfault() thread. IOW, I think data changes before tlb flush but after pte wr-protect is always fine - but that's not fine anymore if the syscall returns. Thanks, -- Peter Xu
