On Wed, Apr 13, 2022 at 02:28:38PM +0200, David Hildenbrand wrote: > On 13.04.22 14:26, Matthew Wilcox wrote: > > On Tue, Apr 12, 2022 at 11:37:09AM +0200, David Hildenbrand wrote: > >> On 12.04.22 10:47, Vlastimil Babka wrote: > >>> There's a VM_BUG_ON_PAGE(PageTransCompound(page), page); later in a > >>> !compound branch. Since compound is now determined by the same check, could > >>> be deleted. > >> > >> Yes, eventually we could get rid of both VM_BUG_ON_PAGE() on both > >> branches and add a single VM_BUG_ON_PAGE(PageTail(page), page) check on > >> the compound branch. (we could also make sure that we're not given a > >> hugetlb page) > > > > As a rule of thumb, if you find yourself wanting to add > > VM_BUG_ON_PAGE(PageTail(page), page), you probably want to change the > > interface to take a folio. > > Yeah, I had the same in mind. Might be a reasonable addon on top -- > although it would stick out in the rmap code a bit because most > functions deal with both, folios and subpages. I have the start of a series which starts looking at the fault path to see where it makes sense to use folios and where it makes sense to use pages. We're (generally) faulting on a PTE, so we need the precise page to be returned in vmf->page. However vmf->cow_page can/should be a folio (because it's definitely not a tail page). That trickles down into copy_present_page() (new_page and prealloc both become folios) and so page_add_new_anon_rmap() then looks like a good target to take a folio. The finish_fault() -> do_set_pte() -> page_add_new_anon_rmap() looks like the only kind of strange place where we don't necessarily have a folio (all the others we just allocated it).
