On Thu, Mar 11, 2021 at 10:21:39AM -0500, Johannes Weiner wrote: > On Thu, Mar 11, 2021 at 09:37:02AM +0100, Michal Hocko wrote: > > Johannes, Hugh, > > > > what do you think about this approach? If we want to stick with > > split_page approach then we need to update the missing place Matthew has > > pointed out. > > I find the __free_pages() code quite tricky as well. But for that > reason I would actually prefer to initiate the splitting in there, > since that's the place where we actually split the page, rather than > spread the handling of this situation further out. Mmm. The thing is, we don't actually split the page because it was never compound. I don't know whether anybody actually does this, but it's legitimate to write: struct page *p = alloc_pages(GFP_KERNEL, 2); free_unref_page(p + 1); free_unref_page(p + 3); free_unref_page(p + 2); __free_page(p); The good news is that I recently made free_unref_page() local to mm/internal.h, so we don't need to worry about device drivers doing this. As far as I can tell, we don't have any exposure to this kind of thing today through functions exported from mm, but I might have missed something. I'd really like to get rid of non-compound high-order pages. Slab, filesystems and anonymous memory all use compound pages. I think it's just crusty old device drivers that don't. And alloc_pages_exact(), of course, but that's kind of internal. > The race condition shouldn't be hot, so I don't think we need to be as > efficient about setting page->memcg_data only on the higher-order > buddies as in Willy's scratch patch. We can call split_page_memcg(), > which IMO should actually help document what's happening to the page. I'm cool with that. I agree, this is not a performance case! > I think that function could also benefit a bit more from step-by-step > documentation about what's going on. The kerneldoc is helpful, but I > don't think it does justice to how tricky this race condition is. Always good to have other people read over your explanation ... the kernel-doc could probably be simplified as a result.
