On Dec 21, 2020, at 1:24 PM, Yu Zhao yuzhao@google.com wrote:
On Mon, Dec 21, 2020 at 12:26:22PM -0800, Linus Torvalds wrote:
On Mon, Dec 21, 2020 at 12:23 PM Nadav Amit nadav.amit@gmail.com wrote:
Using mmap_write_lock() was my initial fix and there was a strong pushback on this approach due to its potential impact on performance.
From whom?
Somebody who doesn't understand that correctness is more important than performance? And that userfaultfd is not the most important part of the system?
The fact is, userfaultfd is CLEARLY BUGGY.
LinusFair enough.
Nadav, for your patch (you might want to update the commit message).
Yes, the commit message is completely off. Will fix.
Thanks for your guidance and assistance.
Reviewed-by: Yu Zhao yuzhao@google.com
While we are all here, there is also clear_soft_dirty() that could use a similar fix…
Let me try to build a small reproducer for clear_soft_dirty() and then I’ll send another patch for it too.
BTW: In general, I think that you are right, and that changing of PTEs should not require taking mmap_lock for write. However, I am not sure cow_user_page() is not the only one that poses a problem and whether a more systematic solution is needed. If cow_user_pages() is the only problem, do you think it is possible to do the copying while holding the PTL? It works for normal-pages, but I am not sure whether special-pages pose special problems.
Anyhow, this is an enhancement that we can try later.
Thanks again, Nadav