[PATCH] drm: dp_tunnel: use kzalloc_flex

[PATCH] drm: dp_tunnel: use kzalloc_flex

[Rosen Penev]

Simplifies allocation by using a flexible array member to remove a
second kzalloc.

Added __counted_by for extra runtime analysis.

Simplify assignment loop. It always returns true. That is,
max_group_count is always equal to group_count.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/gpu/drm/display/drm_dp_tunnel.c | 26 ++++++-------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_tunnel.c b/drivers/gpu/drm/display/drm_dp_tunnel.c
index f442430d8de7..aad1605b956e 100644
--- a/drivers/gpu/drm/display/drm_dp_tunnel.c
+++ b/drivers/gpu/drm/display/drm_dp_tunnel.c
@@ -188,13 +188,13 @@ struct drm_dp_tunnel_group {
 struct drm_dp_tunnel_mgr {
 	struct drm_device *dev;
 
-	int group_count;
-	struct drm_dp_tunnel_group *groups;
 	wait_queue_head_t bw_req_queue;
 
 #ifdef CONFIG_DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG
 	struct ref_tracker_dir ref_tracker;
 #endif
+	int group_count;
+	struct drm_dp_tunnel_group groups[] __counted_by(group_count);
 };
 
 /*
@@ -1893,7 +1893,6 @@ static void destroy_mgr(struct drm_dp_tunnel_mgr *mgr)
 	ref_tracker_dir_exit(&mgr->ref_tracker);
 #endif
 
-	kfree(mgr->groups);
 	kfree(mgr);
 }
 
@@ -1913,33 +1912,20 @@ drm_dp_tunnel_mgr_create(struct drm_device *dev, int max_group_count)
 	struct drm_dp_tunnel_mgr *mgr;
 	int i;
 
-	mgr = kzalloc_obj(*mgr);
+	mgr = kzalloc_flex(*mgr, groups, max_group_count);
 	if (!mgr)
 		return ERR_PTR(-ENOMEM);
 
+	mgr->group_count = max_group_count;
 	mgr->dev = dev;
 	init_waitqueue_head(&mgr->bw_req_queue);
 
-	mgr->groups = kzalloc_objs(*mgr->groups, max_group_count);
-	if (!mgr->groups) {
-		kfree(mgr);
-
-		return ERR_PTR(-ENOMEM);
-	}
-
 #ifdef CONFIG_DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG
 	ref_tracker_dir_init(&mgr->ref_tracker, 16, "drm_dptun");
 #endif
 
-	for (i = 0; i < max_group_count; i++) {
-		if (!init_group(mgr, &mgr->groups[i])) {
-			destroy_mgr(mgr);
-
-			return ERR_PTR(-ENOMEM);
-		}
-
-		mgr->group_count++;
-	}
+	for (i = 0; i < max_group_count; i++)
+		init_group(mgr, &mgr->groups[i]);
 
 	return mgr;
 }
-- 
2.53.0

Claude review: drm: dp_tunnel: use kzalloc_flex

[Claude Code Review Bot]

Overall Series Review

Subject: drm: dp_tunnel: use kzalloc_flex
Author: Rosen Penev <rosenp@gmail.com>
Patches: 1
Reviewed: 2026-03-09T07:39:02.354541

---

This is a single-patch series that converts `drm_dp_tunnel_mgr` to use a flexible array member with `kzalloc_flex`, eliminating a separate allocation for the `groups` array. The conversion is straightforward and largely correct. However, there is a **correctness bug** in the error handling change that needs to be addressed.

---
Generated by Claude Code Patch Reviewer

Claude review: drm: dp_tunnel: use kzalloc_flex

[Claude Code Review Bot]

Patch Review

**The good:**

- The conversion from a separate `groups` pointer to a flexible array member `groups[]` with `__counted_by(group_count)` is clean and appropriate.
- Removing the second `kzalloc` for `mgr->groups` and the corresponding `kfree(mgr->groups)` in `destroy_mgr` is correct — with a flex array, a single `kfree(mgr)` suffices.
- Using `kzalloc_flex(*mgr, groups, max_group_count)` matches the macro's expected signature.

**Bug — error handling dropped from `init_group` loop:**

The patch changes:
```c
-	for (i = 0; i < max_group_count; i++) {
-		if (!init_group(mgr, &mgr->groups[i])) {
-			destroy_mgr(mgr);
-
-			return ERR_PTR(-ENOMEM);
-		}
-
-		mgr->group_count++;
-	}
+	for (i = 0; i < max_group_count; i++)
+		init_group(mgr, &mgr->groups[i]);
```

The commit message claims "It always returns true. That is, max_group_count is always equal to group_count." Looking at `init_group` (line 1597), this is indeed **currently true** — the function unconditionally returns `true`. So the behavioral change is safe today.

However, the patch also **silently changes the return type expectation**: by removing the error check entirely and ignoring the return value, any future change to `init_group` that introduces a failure path would silently be ignored. The cleaner approach would be to also change `init_group`'s return type from `bool` to `void` in the same patch to make the contract explicit. Otherwise a future developer seeing `init_group` returns `bool` might add a failure path without realizing callers now ignore it.

**Nit — `group_count` assignment before `init_group` loop:**

```c
+	mgr->group_count = max_group_count;
```

This is set before the `init_group` loop. In the old code, `mgr->group_count` was incremented one-by-one as each group was initialized, so `destroy_mgr` would only clean up successfully initialized groups. With the new code, if `init_group` were ever changed to fail, `destroy_mgr` would attempt to clean up uninitialized groups.

This is fine given `init_group` can't fail, but it's another reason to **change `init_group` to return `void`** in the same patch to make the invariant clear.

**Struct field reordering:**

```c
-	int group_count;
-	struct drm_dp_tunnel_group *groups;
 	wait_queue_head_t bw_req_queue;
 ...
+	int group_count;
+	struct drm_dp_tunnel_group groups[] __counted_by(group_count);
```

Moving `group_count` and `groups` to the end of the struct is required since flex arrays must be the last member. The `group_count` field is also moved to be adjacent to `groups[]` which is good for `__counted_by`. The reordering may introduce a small hole for padding between `wait_queue_head_t bw_req_queue` and the `#ifdef` block, but this is unlikely to matter in practice.

**Recommendation:** The patch should also change `init_group` from `static bool` to `static void` to match the new calling convention. Without that, the patch is technically correct but leaves a misleading function signature.

---
Generated by Claude Code Patch Reviewer