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.
