Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Evicted VRAM BOs bypass memory cgroup (memcg) accounting because TTM_PL_FLAG_MEMCG is explicitly cleared during eviction. --
commit 05e0eb5ded826da3ea51ec14b35acb22f46c4aaa Author: Dave Airlie <[email protected]> amdgpu: add support for memory cgroups This adds support for adding a obj cgroup to a buffer object, and passing in the placement flags to make sure it's accounted properly. [ ... ] > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 8062b3d611574..1c8123c564ac1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -155,11 +155,14 @@ static void amdgpu_evict_flags(struct ttm_buffer_object > *bo, > amdgpu_bo_placement_from_domain(abo, > AMDGPU_GEM_DOMAIN_GTT | > AMDGPU_GEM_DOMAIN_CPU); > } > + for (int i = 0; i < abo->placement.num_placement; i++) > + abo->placements[i].flags &= ~TTM_PL_FLAG_MEMCG; [Severity: High] Does clearing TTM_PL_FLAG_MEMCG here allow an application to bypass memcg limits? If a container continually allocates VRAM BOs, older BOs will be evicted to system memory. Because the TTM_PL_FLAG_MEMCG flag is explicitly removed from the placement flags in amdgpu_evict_flags(), it appears that the system memory pages allocated during this eviction will not be charged to the application's memcg, potentially allowing unbounded host memory consumption. > break; > case TTM_PL_TT: > case AMDGPU_PL_PREEMPT: > default: > amdgpu_bo_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_CPU); > + abo->placements[0].flags &= ~TTM_PL_FLAG_MEMCG; [Severity: High] Does this similarly drop memcg accounting when evicting other memory types to the CPU domain? > break; > } > *placement = abo->placement; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
