Re: [PATCH 3/5] mm: Remove references to folio in split_page_memcg()

[Matthew Wilcox <willy@infradead.org>]

On Thu, Mar 13, 2025 at 12:01:14PM -0400, Johannes Weiner wrote:
> On Thu, Mar 13, 2025 at 02:58:52PM +0000, Matthew Wilcox (Oracle) wrote:
> > @@ -2697,6 +2697,23 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp,
> >  	return ret;
> >  }
> >  
> > +static struct obj_cgroup *page_objcg(const struct page *first)
> 
> "first" is odd for that function. "page"?

I'm trying to indicate that it's the first page of the allocation (head
page is wrong because it's not a compound allocation).  But I can just
call it 'page'.  We do that in enough places ...

> > @@ -3091,16 +3107,16 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
> >   */
> >  void split_page_memcg(struct page *first, unsigned order)
> >  {
> > -	struct folio *folio = page_folio(first);
> > +	struct obj_cgroup *objcg = page_objcg(first);
> >  	unsigned int i, nr = 1 << order;
> >  
> > -	if (mem_cgroup_disabled() || !folio_memcg_charged(folio))
> > +	if (!objcg)
> >  		return;
> 
> mem_cgroup_disabled() is a jump label, so a bit cheaper than the
> branch. Can you keep that and do page_objcg() after it?

Sure, I can do that.  I didn't think it was really worthwhile given
how common splitting is (very rare) and we've already incurred the
function call overhead.