Hi Pavel, On Wed, Mar 18, 2015 at 10:34:26PM +0300, Pavel Emelyanov wrote: > Hi, > > On the recent LSF Andrea presented his userfault-fd patches and > I had shown some issues that appear in usage scenarios when the > monitor task and mm task do not cooperate to each other on VM > changes (and fork()-s). > > Here's the implementation of the extended uffd API that would help > us to address those issues. > > As proof of concept I've implemented only fix for fork() case, but > I also plan to add the mremap() and exit() notifications, both are > also required for such non-cooperative usage. > > More details about the extension itself is in patch #2 and the fork() > notification description is in patch #3. > > Comments and suggestion are warmly welcome :) This looks feasible. > Andrea, what's the best way to go on with the patches -- would you > prefer to include them in your git tree or should I instead continue > with them on my own, re-sending them when required? Either way would > be OK for me. Ok so various improvements happened in userfaultfd patchset over the last month so your incremental patchset likely requires a rebase sorry. When you posted it I was in the middle of the updates. Now things are working stable and I have no pending updates, so it would be a good time for a rebase. I can merge it if you like, it's your call if you prefer to maintain it incrementally or if I should merge it, but I wouldn't push it to Andrew for upstream integration in the first batch, as this complicates things further and it's not fully complete yet (at least the version posted only handled fork). I think it can be merged incrementally in a second stage. The major updates of the userfaultfd patchset over the last month were: 1) Various mixed fixes thanks to the feedback from Dave Hansen and David Gilbert. The most notable one is the use of mm_users instead of mm_count to pin the mm to avoid crashes that assumed the vma still existed (in the userfaultfd_release method and in the various ioctl). exit_mmap doesn't even set mm->mmap to NULL, so unless I introduce a userfaultfd_exit to call in mmput, I have to pin the mm_users to be safe. This is mainly an issue for the non-cooperative usage you're implementing. Can you catch the exit somehow so you can close the fd? The memory won't be released until you do. Is this ok with you? I suppose you had to close the fd somehow anyway. 2) userfaults are waken immediately even if they're not been "read" yet, this can lead to POLLIN false positives (so I only allow poll if the fd is open in nonblocking mode to be sure it won't hang). Is it too paranoid to return POLLERR if the fd is not open in nonblocking mode? http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=f222d9de0a5302dc8ac62d6fab53a84251098751 3) optimize read to return entries in O(1) and poll which was already O(1) becomes lockless. This required to split the waitqueue in two, one for pending faults and one for non pending faults, and the faults are refiled across the two waitqueues when they're read. Both waitqueues are protected by a single lock to be simpler and faster at runtime (the fault_pending_wqh one). http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=9aa033ed43a1134c2223dac8c5d9e02e0100fca1 4) Allocate the ctx with kmem_cache_alloc. This is going to collide a bit with your cleanup patch sorry. http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=f5a8db16d2876eed8906a4d36f1d0e06ca5490f6 5) Originally qemu had two bitflags for each page and kept 3 states (of the 4 possible with two bits) for each page in order to deal with the races that can happen if one thread is reading the userfaults and another thread is calling the UFFDIO_COPY ioctl in the background. This patch solves all races in the kernel so the two bits per page can be dropped from qemu codebase. I started documenting the races that can materialize by using 2 threads (instead of running the workload single threaded with a single poll event loop), and how userland had to solve them until I decided it was simpler to fix the race in the kernel by running an ad-hoc pagetable walk inside the wait_event()-kind-of-section. This simplified qemu significantly and it doesn't make the kernel much more complicated. I tried this before in much older versions but I use gup_fast for it and it didn't work well with gup_fast for various reasons. http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=41efeae4e93f0296436f2a9fc6b28b6b0158512a After this patch the only reason to call UFFDIO_WAKE is to handle the userfaults in batches in combination with the DONT_WAKE flag of UFFDIO_COPY. 6) I removed the read recursion from mcopy_atomic. This avoids to depend on the write-starvation behavior of rwsem to be safe. After this change the rwsem is free to stop any further down_read if there's a down_write waiting on the lock. http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=b1e3a08acc9e3f6c2614e89fc3b8e338daa58e18 About other troubles for the non cooperative usage: MADV_DONTNEED likely needs to be trapped too or how do you know that you shall map a zero page instead of the old data at the faulting address? Thanks, Andrea -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>