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"?
> @@ -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?
> for (i = 1; i < nr; i++)
> - folio_page(folio, i)->memcg_data = folio->memcg_data;
> + page_set_objcg(&first[i], objcg);
>
> - obj_cgroup_get_many(__folio_objcg(folio), nr - 1);
> + obj_cgroup_get_many(objcg, nr - 1);
> }
This is much nicer indeed. Thanks for doing this.
With the above nits considered,
Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
