> On Jan 27, 2022, at 7:20 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 1/27/22 01:02, Chris Mason wrote: >> From the btrfs side, bare calls to set_page_dirty() are suboptimal, >> since it doesn’t go through the ->page_mkwrite() dance that we use to >> properly COW things. It’s still much better than SetPageDirty(), but >> I’d love to understand why kvm needs to dirty the page so we can >> figure out how to go through the normal mmap file io paths. > Shouldn't ->page_mkwrite() occur at the point of get_user_pages, such as via handle_mm_fault->handle_pte_fault->do_fault->do_shared_fault? That always happens before SetPageDirty(), or set_page_dirty() after Boris's patch. page_mkwrite() is where btrfs does its COW setup, waits for IO in flight, and also sets the page dirty. If that’s already happening for these pages, do we need an additional set_page_dirty() at all? Boris found https://lists.openwall.net/linux-kernel/2016/02/11/702, where Maxim suggests just dropping the SetPageDirty() on file back pages. The problem with bare set_page_dirty() calls is that it bypasses our synchronization for stable pages. We have to support it because of weird get_user_pages() corners, but page_mwkrite() is much preferred. Hopefully our use of clear_page_dirty_for_io() makes sure that any modifications to the page go through page_mkwrite() again, so I think Maxim’s patch might just be correct. -chris
