On Tue, Dec 22, 2020 at 11:20:21AM -0800, Nadav Amit wrote:
On Dec 22, 2020, at 10:30 AM, Yu Zhao yuzhao@google.com wrote:
On Tue, Dec 22, 2020 at 04:40:32AM -0800, Nadav Amit wrote:
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).
Reviewed-by: Yu Zhao yuzhao@google.com
While we are all here, there is also clear_soft_dirty() that could use a similar fix…
Just an update as for why I have still not sent v2: I fixed clear_soft_dirty(), created a reproducer, and the reproducer kept failing.
So after some debugging, it appears that clear_refs_write() does not flush the TLB. It indeed calls tlb_finish_mmu() but since 0758cd830494 ("asm-generic/tlb: avoid potential double flush”), tlb_finish_mmu() does not flush the TLB since there is clear_refs_write() does not call to __tlb_adjust_range() (unless there are nested TLBs are pending).
Sorry Nadav, I assumed you knew this existing problem fixed by: https://patchwork.kernel.org/project/linux-mm/cover/20201210121110.10094-1-w...
Thanks, Yu! For some reason I assumed it was already upstreamed and did not look back (yet if I was cc’d on v2…)
I'll repost in the new year, as it was a bit tight for the merge window. I've made a note to put you on cc.
Yet, something still goes bad. Debugging.
Did you figure this out? I tried to read the whole thread, but it's a bit of a rollercoaster.
Will