Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The memcg-aware shrinker count callback returns the global node pool size instead of the memcg-specific pool size, severely distorting proportional reclaim. - [High] DMA-allocated pages permanently corrupt NR_GPU_ACTIVE and NR_GPU_RECLAIM vmstat counters when added to and freed from the TTM pool. -- commit 6e92a3554c189b1a664eb6626ab6f741e3bdc804 Author: Dave Airlie <[email protected]> ttm: add support for a module option to disable memcg integration This adds a kconfig and a module option to turn off ttm memcg integration completely. When this is used, no object will ever end up using memcg aware paths. > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index 01b6ff2d81444..b7263d26c9902 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c [ ... ] > @@ -321,7 +339,7 @@ static void ttm_pool_type_give(struct ttm_pool_type *pt, > struct page *p) > > INIT_LIST_HEAD(&p->lru); > rcu_read_lock(); > - list_lru_add(&pt->pages, &p->lru, nid, page_memcg_check(p)); > + list_lru_add(&pt->pages, &p->lru, nid, ttm_memcg ? page_memcg_check(p) > : NULL); > rcu_read_unlock(); > > atomic_long_add(num_pages, &allocated_pages[nid]); [Severity: High] This is a pre-existing issue, but does this path corrupt the NR_GPU_ACTIVE and NR_GPU_RECLAIM vmstat counters for DMA-allocated pages? In ttm_pool_alloc_page(), pages allocated via the DMA API bypass the mem_cgroup_charge_gpu_page() call, so the NR_GPU_ACTIVE counter is never incremented. However, when these DMA pages are returned to the pool here in ttm_pool_type_give(), the code unconditionally updates the stats by calling mem_cgroup_move_gpu_page_reclaim(NULL, p, pt->order, true). Because the page has no tracked memcg, this subtracts from the global node's NR_GPU_ACTIVE (causing an underflow) and adds to NR_GPU_RECLAIM. Furthermore, when the DMA page is ultimately freed from the pool: drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_free_page() { if (!pool || !ttm_pool_uses_dma_alloc(pool)) { __free_pages_gpu_account(p, order, reclaim); return; } ... dma_free_attrs(...); } It skips __free_pages_gpu_account() and is freed directly. Thus, NR_GPU_RECLAIM is never decremented, causing a permanent and unbounded leak of the global reclaim counter on the node. [ ... ] [Severity: High] This is a pre-existing issue, but does the memcg-aware shrinker count callback severely distort proportional reclaim by returning the global pool size? When the VM reclaims memory under a specific memory cgroup, it calls ttm_pool_shrinker_count() with sc->memcg set: drivers/gpu/drm/ttm/ttm_pool.c:ttm_pool_shrinker_count() { unsigned long num_pages = atomic_long_read(&allocated_pages[sc->nid]); return num_pages ? num_pages : SHRINK_EMPTY; } This unconditionally returns the global number of allocated pages on the node instead of the number of pages actually in the specified memcg's LRU list. Could this cause the VM to vastly overestimate the amount of reclaimable memory for any memcg under pressure (potentially reporting gigabytes of global pool pages as reclaimable by a small cgroup)? It seems this would skew the proportional reclaim heuristics, causing the VM to apply excessive pressure to this shrinker while under-pressuring other memory caches. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
