Claude review: cgroup/dmem: introduce a peak file

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

Patch Review

**Correctness: Good**

The core implementation is sound:

```c
static u64 get_resource_peak(struct dmem_cgroup_pool_state *pool)
{
	return pool ? READ_ONCE(pool->cnt.watermark) : 0;
}
```

This correctly handles the NULL pool case (returns 0, matching the `get_resource_current` pattern) and uses `READ_ONCE` to read the racy watermark, which is the established pattern in `mm/page_counter.c` (line 103: `if (new > READ_ONCE(c->watermark))`).

The watermark is updated hierarchically in `page_counter_try_charge()` and `page_counter_charge()`, walking the parent chain, so the "cgroup and its descendants" claim in the documentation is correct — charging to a leaf updates the watermark on all ancestors.

**Flag choice: Correct**

```c
{
	.name = "peak",
	.seq_show = dmem_cgroup_region_peak_show,
	.flags = CFTYPE_NOT_ON_ROOT,
},
```

Using `CFTYPE_NOT_ON_ROOT` is consistent with `min`, `low`, and `max` in this controller. The root cgroup's peak is not meaningful (it would just track total system device memory usage, which `capacity` already covers from a different angle).

**Nit — v3 changelog has EDITME placeholders:**

```
Changes in v3:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
```

This is a b4 template artifact that wasn't filled in. It doesn't affect the code, but it's sloppy and should be fixed before the patch is applied. The maintainer will likely ask for a resend.

**Nit — Documentation placement:**

The `dmem.peak` documentation is placed between `dmem.current` and `dmem.capacity`, which is a good logical grouping (current/peak are related usage metrics). The text is clear and mirrors the `memory.peak` description style.

**Observation — No selftest:**

The patch doesn't include a selftest. While there's no existing dmem selftest infrastructure to extend, a simple test (create cgroup, allocate device memory, read peak, verify peak >= current) would be valuable, especially since peak tracking is the kind of feature that could silently regress.

**Observation — Future write-to-reset:**

The commit message mentions "for now, make it read-only." When write-to-reset is added later, it will likely need to use `local_watermark` (like `memory.peak` does for per-FD resets via `page_counter_reset_watermark()`). The current implementation reading `watermark` (the global one) is correct for a read-only peak file, so no changes are needed now.

**Overall: The patch is a clean, minimal, and correct addition. Fix the EDITME changelog and consider adding a selftest, otherwise this is ready.**

---
Generated by Claude Code Patch Reviewer