Claude review: Re: [PATCH] cgroup/dmem: return -ENOMEM on failed pool preallocation

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctness: Correct.**

Looking at the function flow (lines 585–606 in the current tree):

```c
while (!pool) {
    spin_lock(&dmemcg_lock);
    if (!region->unregistered)
        pool = get_cg_pool_locked(cg, region, &allocpool);
    else
        pool = ERR_PTR(-ENODEV);
    ...
    spin_unlock(&dmemcg_lock);

    if (pool == ERR_PTR(-ENOMEM)) {
        pool = NULL;
        if (WARN_ON(allocpool))
            continue;

        allocpool = kzalloc_obj(*allocpool);
        if (allocpool) {
            pool = NULL;
            continue;
        }
        // <--- BUG: pool is NULL here, loop retries forever
    }
}
```

When `kzalloc_obj()` returns NULL (allocation failure), execution falls through the `if (allocpool)` block without setting `pool`. Since `pool` was set to NULL at line 596 (`pool = NULL`), the `while (!pool)` condition is still true, and the loop retries. But `allocpool` is also still NULL, so the next iteration will call `get_cg_pool_locked()` without a preallocated pool, which will again return `ERR_PTR(-ENOMEM)`, and the cycle repeats — an infinite loop under memory pressure, exactly when you'd least want it.

The fix:

```c
        allocpool = kzalloc_obj(*allocpool);
        if (allocpool) {
            pool = NULL;
            continue;
        }
+       pool = ERR_PTR(-ENOMEM);
    }
```

This causes the `while (!pool)` loop to terminate (since `ERR_PTR(-ENOMEM)` is non-NULL), and `pool` is returned to the caller. Since callers already check for `IS_ERR()` returns (the `-ENODEV` path at line 590 already sets an ERR_PTR), this is consistent with the existing error contract.

**Minor observations:**

1. The `Fixes:` tag references the correct commit that introduced the dmem cgroup code.
2. The `kfree(allocpool)` at line 608 is safe since `allocpool` is NULL in this error path.
3. One could argue that `GFP_KERNEL` allocation failure should perhaps use `__GFP_NOFAIL` or retry with `GFP_NOIO` depending on context, but simply propagating the error is the safest and most standard approach, and matches how `get_cg_pool_locked` already signals `-ENOMEM`.

**No issues found. Clean, minimal, correct fix.**

---
Generated by Claude Code Patch Reviewer