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

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Mar 13, 2025 at 05:59:15PM +0000, Matthew Wilcox wrote:
> On Thu, Mar 13, 2025 at 12:01:14PM -0400, Johannes Weiner wrote:
> > >  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?
> 
> I think this might be better?
> 
> -static struct obj_cgroup *page_objcg(const struct page *first)
> +static struct obj_cgroup *page_objcg(const struct page *page)
>  {
> -       unsigned long memcg_data = first->memcg_data;
> +       unsigned long memcg_data = page->memcg_data;
> 
> -       if (!memcg_data)
> +       if (mem_cgroup_disabled() || !memcg_data)
>                 return NULL;
> 
> I'd expect the compiler to inline page_objcg() into its caller and
> optimise away the NULL test if mem_cgroup_disabled().

That sounds good to me. Thanks!