On 12/09/2017 14:52, David Hildenbrand wrote: > mm/page-writeback.c: It only seems to be relevant for !anonymous mappings. > > "set_page_dirty() is racy [...] This is because another CPU could > truncate the page off the mapping and then free the mapping." > > "For pages with a mapping this should be done under the page lock for > the benefit of asynchronous memory errors who prefer a consistent dirty > state. This rule can be broken in some special cases [...]" > > Sounds to me like the worst thing that could happen is losing a dirty > flag. Which should not matter if somebody tries to do nasty stuff with > the mapping either way. > > > But to verify: Paolo, can you recall why we don't need the _locked > variants in kvm common code? That predates me by several years, but if you look at set_page_dirty it is if (page_mapping(page)) return ...->set_page_dirty(page); else return TestSetPageDirty(page); so KVM basically expects that !page_mapping(page) or that the page is ram-backed. That includes a hugetlbfs page, for which the callback is *also* doing just SetPageDirty, or a few non-anonymous maps for which the callback is __set_page_dirty_no_writeback; these in turn are ramfs and tmpfs. If you map files to KVM, the page cache (fs/buffer.c) is not going to know that the pages are dirty, and likewise the file system is not going to know that the inode is dirty. So don't do that. Paolo
