On Wed, May 4, 2011 at 5:09 PM, Michel Lespinasse wrote: > > FYI, the attached code causes an infinite loop in kernels that have > the 95042f9eb7 commit: Mmm. Yes. The atomic fault will never work, and the get_user_pages() thing won't either, so things will just loop forever. > Linus, I am not sure as to what would be the preferred way to fix > this. One option could be to modify fault_in_user_writeable so that it > passes a non-NULL page pointer, and just does a put_page on it > afterwards. While this would work, this is kinda ugly and would slow > down futex operations somewhat. No, that's just ugly as hell. > A more conservative alternative could > be to enable the guard page special case under an new GUP flag, but > this loses much of the elegance of your original proposal... How about only doing that only for FOLL_MLOCK? Also, looking at mm/mlock.c, why _do_ we call get_user_pages() even if the vma isn't mlocked? That looks bogus. Since we have dropped the mm_semaphore, an unlock may have happened, and afaik we should *not* try to bring those pages back in at all. There's this whole comment about that in the caller ("__mlock_vma_pages_range() double checks the vma flags, so that it won't mlock pages if the vma was already munlocked."), but despite that it would actually call __get_user_pages() even if the VM_LOCKED bit had been cleared (it just wouldn't call it with the FOLL_MLOCK flag). So maybe something like the attached? UNTESTED! And maybe there was some really subtle reason to still call __get_user_pages() without that FOLL_MLOCK thing that I'm missing. Linus