Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Faking `MEMCG_DATA_KMEM` without charging the `MEMCG_KMEM` stat causes state corruption and potential counter underflow. - [High] `mem_cgroup_move_gpu_page_reclaim` ignores the `to_reclaim` argument during cross-cgroup moves, corrupting the active/reclaim stats. - [High] The logic condition in `mem_cgroup_move_gpu_page_reclaim` skips charge transfers for uncharged pages, bypassing memory limits. - [High] Charging a GPU page to the root memcg triggers a `VM_WARN_ON_ONCE` and permanently leaks memory usage counters. --
commit 1761466dad501528141242e7d0630e4e61d30d71 Author: Dave Airlie <[email protected]> Date: Mon Jul 6 05:22:30 2026 +0000 memcg: add support for GPU page counters. (v5) This introduces 2 new statistics and 3 new memcontrol APIs for dealing with GPU system memory allocations. The stats corresponds to the same stats in the global vmstat, for number of active GPU pages, and number of pages in pools that can be reclaimed. > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 6dc4888a90f3f..4c682b91cbbee 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c [ ... ] > @@ -5508,6 +5512,106 @@ void mem_cgroup_flush_workqueue(void) > } > > +/** > + * mem_cgroup_charge_gpu_page - charge a page to GPU memory tracking > + * @objcg: objcg to charge, NULL charges root memcg > + * @page: page to charge > + * @order: page allocation order > + * @gfp_mask: gfp mode > + * @reclaim: charge the reclaim counter instead of the active one. > + * > + * Charge the order sized @page to the objcg. Returns %true if the charge > fit within > + * @objcg's configured limit, %false if it doesn't. > + */ > +bool mem_cgroup_charge_gpu_page(struct obj_cgroup *objcg, struct page *page, > + unsigned int order, gfp_t gfp_mask, bool > reclaim) > +{ > + unsigned int nr_pages = 1 << order; > + struct mem_cgroup *memcg = NULL; > + struct lruvec *lruvec; > + int ret; > + > + if (objcg) { > + memcg = get_mem_cgroup_from_objcg(objcg); > + > + ret = try_charge_memcg(memcg, gfp_mask, nr_pages); [Severity: High] Does this trigger a kernel warning and counter leak for root processes? When a root process allocates GPU memory, current_obj_cgroup() can return a valid obj_cgroup for the root memcg. Calling try_charge_memcg() on the root memcg succeeds and subsequently calls refill_stock(). Inside refill_stock(), this triggers VM_WARN_ON_ONCE(mem_cgroup_is_root(memcg)). Additionally, mem_cgroup_uncharge_gpu_page() explicitly skips refill_stock() if it is the root memcg. Since the drain mechanism is used to uncharge the counters in this API, does this permanently leak the root memcg's page counters? > + if (ret) { > + mem_cgroup_put(memcg); > + return false; > + } > + > + obj_cgroup_get(objcg); > + page_set_objcg(page, objcg); [Severity: High] Will this cause MEMCG_KMEM counter underflow and state corruption? page_set_objcg() overwrites page->memcg_data with objcg | MEMCG_DATA_KMEM. However, unlike standard kmem paths using obj_cgroup_charge_pages(), this function does not charge the MEMCG_KMEM counter. If a driver leaks the page (failing to uncharge it), the normal page allocator will eventually process it. free_pages_prepare() sees PageMemcgKmem(page) is true and calls __memcg_kmem_uncharge_page(), which decrements MEMCG_KMEM. Since it was never incremented, this causes a permanent underflow. Also, if this exported API is mistakenly used on pages that already have tracking (like shmem), does this silently corrupt their memcg_data invariants? > + } > + > + lruvec = mem_cgroup_lruvec(memcg, page_pgdat(page)); > + mod_lruvec_state(lruvec, reclaim ? NR_GPU_RECLAIM : NR_GPU_ACTIVE, > nr_pages); > + > + mem_cgroup_put(memcg); > + return true; > +} > +EXPORT_SYMBOL_GPL(mem_cgroup_charge_gpu_page); [ ... ] > +/** > + * mem_cgroup_move_gpu_reclaim - move pages from gpu to gpu reclaim and back > + * @new_objcg: objcg to move page to, NULL if just stats update. > + * @nr_pages: number of pages to move > + * @to_reclaim: true moves pages into reclaim, false moves them back > + */ > +bool mem_cgroup_move_gpu_page_reclaim(struct obj_cgroup *new_objcg, > + struct page *page, > + unsigned int order, > + bool to_reclaim) > +{ > + struct obj_cgroup *objcg = page_objcg(page); > + > + if (!objcg || !new_objcg || objcg == new_objcg) { [Severity: High] Does this skip charge transfers for uncharged pages, allowing memory limit bypass? If a page was allocated without memcg accounting (e.g., by a system process or during early boot), its objcg is NULL. When a container (with a valid new_objcg) allocates this page from the global GPU pool, the !objcg condition evaluates to true. The function enters this block, toggles stats locally, and returns true, skipping the actual charge transfer to new_objcg. Does this allow a container to use the GPU page without it being charged against its memcg memory limit? > + struct mem_cgroup *memcg = objcg ? > get_mem_cgroup_from_objcg(objcg) : NULL; > + struct lruvec *lruvec; > + unsigned long flags; > + int nr_pages = 1 << order; > + > + lruvec = mem_cgroup_lruvec(memcg, page_pgdat(page)); > + local_irq_save(flags); > + mod_lruvec_state(lruvec, to_reclaim ? NR_GPU_RECLAIM : > NR_GPU_ACTIVE, nr_pages); > + mod_lruvec_state(lruvec, to_reclaim ? NR_GPU_ACTIVE : > NR_GPU_RECLAIM, -nr_pages); > + local_irq_restore(flags); > + mem_cgroup_put(memcg); > + return true; > + } else { > + mem_cgroup_uncharge_gpu_page(page, order, true); > + return mem_cgroup_charge_gpu_page(new_objcg, page, order, 0, > false); [Severity: High] Does this ignore the to_reclaim argument during cross-cgroup moves? This unconditionally calls mem_cgroup_uncharge_gpu_page() with reclaim=true and mem_cgroup_charge_gpu_page() with reclaim=false. This hardcodes the transition as moving from the RECLAIM state to the ACTIVE state. If a caller attempts to move a page between obj_cgroups while keeping its state as ACTIVE, wouldn't this erroneously uncharge the old cgroup's RECLAIM state (causing an underflow) and charge the new cgroup's ACTIVE state? > + } > +} > +EXPORT_SYMBOL_GPL(mem_cgroup_move_gpu_page_reclaim); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
