Messages in this thread | | | Date | Wed, 13 Apr 2022 13:48:51 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH v3 08/16] mm/rmap: drop "compound" parameter from page_add_new_anon_rmap() |
| |
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).
| |