[PATCH] drm/nouveau: Wire up dmem cgroups

[PATCH] drm/nouveau: Wire up dmem cgroups

[Natalie Vock]

Userspace can now make use of memory protection via dmem cgroups. Let
nouveau benefit from this as well by registering the vram region with
the dmem cgroup controller.

This patch adapts the approach amdgpu and Xe have taken for enabling
dmem cgroups.

Signed-off-by: Natalie Vock <natalie.vock@gmx.de>
---
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index ad01f922aa86a..860bca7e3ce28 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -188,6 +188,11 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm)
 
 		man->func = &nouveau_vram_manager;
 
+		man->cg = drmm_cgroup_register_region(drm->dev, "vram",
+						      drm->gem.vram_available);
+		if (IS_ERR(man->cg))
+			return PTR_ERR(man->cg);
+
 		ttm_resource_manager_init(man, &drm->ttm.bdev,
 					  drm->gem.vram_available >> PAGE_SHIFT);
 		ttm_set_driver_manager(&drm->ttm.bdev, TTM_PL_VRAM, man);
-- 
2.53.0

Claude review: drm/nouveau: Wire up dmem cgroups

[Claude Code Review Bot]

Overall Series Review

Subject: drm/nouveau: Wire up dmem cgroups
Author: Natalie Vock <natalie.vock@gmx.de>
Patches: 1
Reviewed: 2026-04-12T10:21:18.385961

---

This is a single, small patch that wires up dmem cgroup support for the nouveau driver, following the established pattern from amdgpu and Xe. The approach is correct in spirit — register the VRAM region with `drmm_cgroup_register_region()` and store the result in `man->cg`. However, there is a memory leak bug on the error path that must be fixed before merging.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/nouveau: Wire up dmem cgroups

[Claude Code Review Bot]

Patch Review

**Concept: Correct.** The registration is placed in the right location within `nouveau_ttm_init_vram()`, on the Tesla+ path where `man` is dynamically allocated. The header for `drmm_cgroup_register_region` is available transitively via `nouveau_drv.h` -> `drm/drm_drv.h`. The region name `"vram"` and size `drm->gem.vram_available` are appropriate and consistent with what amdgpu/xe do.

**Bug: Memory leak of `man` on cgroup registration failure.**

Looking at the patched code (lines 183–194):

```c
if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA) {
    struct ttm_resource_manager *man = kzalloc_obj(*man);

    if (!man)
        return -ENOMEM;

    man->func = &nouveau_vram_manager;

    man->cg = drmm_cgroup_register_region(drm->dev, "vram",
                          drm->gem.vram_available);
    if (IS_ERR(man->cg))
        return PTR_ERR(man->cg);
```

If `drmm_cgroup_register_region()` fails, the function returns immediately via `return PTR_ERR(man->cg)` without freeing `man`, which was allocated with `kzalloc_obj()` at line 184. This is a memory leak.

By contrast, in the amdgpu driver (`amdgpu_vram_mgr.c:917-923`), `man` is embedded within `struct amdgpu_vram_mgr` (not separately heap-allocated), so there's no analogous leak. The nouveau code allocates `man` on the heap and must free it on all error paths.

**Fix:** Add `kfree(man)` before the error return:

```c
    if (IS_ERR(man->cg)) {
        kfree(man);
        return PTR_ERR(man->cg);
    }
```

Note: there is a subtlety — `man->cg` is read after `kfree(man)` in `PTR_ERR(man->cg)`. To be safe, this should use a local variable:

```c
    if (IS_ERR(man->cg)) {
        int ret = PTR_ERR(man->cg);
        kfree(man);
        return ret;
    }
```

**Pre-Tesla path:** The `else` branch (line 201–203) uses `ttm_range_man_init()` which handles its own allocation internally. There is no cgroup registration for this path, which seems intentional and reasonable (pre-Tesla hardware is quite old).

**No-op when cgroups disabled:** `drmm_cgroup_register_region()` should return a non-error value when `CONFIG_CGROUP_DMEM` is not enabled (a stub), so this won't break builds without cgroup support. This is fine.

**Summary:** The patch is the right approach but needs the `kfree(man)` leak fix on the error path. With that addressed, this is a clean, minimal addition. Worth also considering whether a `Cc: stable` tag is warranted (probably not — this is new functionality, not a fix).

---
Generated by Claude Code Patch Reviewer