[PATCH] accel/habanalabs: kzalloc + kcalloc to kzalloc_flex

[PATCH] accel/habanalabs: kzalloc + kcalloc to kzalloc_flex

[Rosen Penev]

Simplifies allocation by using a flexible array member.

No need to free separately.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 .../habanalabs/common/command_submission.c    | 20 ++-----------------
 drivers/accel/habanalabs/common/habanalabs.h  |  2 +-
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/accel/habanalabs/common/command_submission.c b/drivers/accel/habanalabs/common/command_submission.c
index ba4257bda77b..ba99329eacbd 100644
--- a/drivers/accel/habanalabs/common/command_submission.c
+++ b/drivers/accel/habanalabs/common/command_submission.c
@@ -809,7 +809,6 @@ static void cs_do_release(struct kref *ref)

 	hl_fence_put(cs->fence);

-	kfree(cs->jobs_in_queue_cnt);
 	kfree(cs);
 }

@@ -907,9 +906,9 @@ static int allocate_cs(struct hl_device *hdev, struct hl_ctx *ctx,

 	cntr = &hdev->aggregated_cs_counters;

-	cs = kzalloc_obj(*cs, GFP_ATOMIC);
+	cs = kzalloc_flex(*cs, jobs_in_queue_cnt, hdev->asic_prop.max_queues, GFP_ATOMIC);
 	if (!cs)
-		cs = kzalloc_obj(*cs);
+		cs = kzalloc_flex(*cs, jobs_in_queue_cnt, hdev->asic_prop.max_queues);

 	if (!cs) {
 		atomic64_inc(&ctx->cs_counters.out_of_mem_drop_cnt);
@@ -947,19 +946,6 @@ static int allocate_cs(struct hl_device *hdev, struct hl_ctx *ctx,
 		goto free_cs;
 	}

-	cs->jobs_in_queue_cnt = kcalloc(hdev->asic_prop.max_queues,
-			sizeof(*cs->jobs_in_queue_cnt), GFP_ATOMIC);
-	if (!cs->jobs_in_queue_cnt)
-		cs->jobs_in_queue_cnt = kcalloc(hdev->asic_prop.max_queues,
-				sizeof(*cs->jobs_in_queue_cnt), GFP_KERNEL);
-
-	if (!cs->jobs_in_queue_cnt) {
-		atomic64_inc(&ctx->cs_counters.out_of_mem_drop_cnt);
-		atomic64_inc(&cntr->out_of_mem_drop_cnt);
-		rc = -ENOMEM;
-		goto free_cs_cmpl;
-	}
-
 	cs_cmpl->hdev = hdev;
 	cs_cmpl->type = cs->type;
 	spin_lock_init(&cs_cmpl->lock);
@@ -1014,8 +1000,6 @@ static int allocate_cs(struct hl_device *hdev, struct hl_ctx *ctx,

 free_fence:
 	spin_unlock(&ctx->cs_lock);
-	kfree(cs->jobs_in_queue_cnt);
-free_cs_cmpl:
 	kfree(cs_cmpl);
 free_cs:
 	kfree(cs);
diff --git a/drivers/accel/habanalabs/common/habanalabs.h b/drivers/accel/habanalabs/common/habanalabs.h
index d94c2ba22a6a..dc13c8a6f242 100644
--- a/drivers/accel/habanalabs/common/habanalabs.h
+++ b/drivers/accel/habanalabs/common/habanalabs.h
@@ -2046,7 +2046,6 @@ struct hl_userptr {
  * @encaps_signals: true if this CS has encaps reserved signals.
  */
 struct hl_cs {
-	u16			*jobs_in_queue_cnt;
 	struct hl_ctx		*ctx;
 	struct list_head	job_list;
 	spinlock_t		job_lock;
@@ -2080,6 +2079,7 @@ struct hl_cs {
 	u8			staged_cs;
 	u8			skip_reset_on_timeout;
 	u8			encaps_signals;
+	u16			jobs_in_queue_cnt[];
 };

 /**
--
2.53.0

Claude review: accel/habanalabs: kzalloc + kcalloc to kzalloc_flex

[Claude Code Review Bot]

Overall Series Review

Subject: accel/habanalabs: kzalloc + kcalloc to kzalloc_flex
Author: Rosen Penev <rosenp@gmail.com>
Patches: 1
Reviewed: 2026-03-11T12:54:50.765966

---

This is a single-patch series that converts the `jobs_in_queue_cnt` member of `struct hl_cs` from a separately allocated pointer to a flexible array member (FAM), using the `kzalloc_flex()` API. The conversion is straightforward and correct in principle — it removes a separate `kcalloc` allocation and its corresponding `kfree`, simplifying the code and reducing the number of allocations.

The patch is **generally correct** but has a few issues worth noting.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/habanalabs: kzalloc + kcalloc to kzalloc_flex

[Claude Code Review Bot]

Patch Review

**Positive aspects:**

- The conversion to a flexible array member is a valid cleanup. It eliminates a separate allocation, a separate free, and an error-handling path (`free_cs_cmpl` label).
- The `kzalloc_flex` API is used correctly — the macro takes `(VAR_OR_TYPE, FAM, COUNT, GFP_FLAGS)`.
- The GFP_ATOMIC-then-GFP_KERNEL fallback pattern is preserved.
- All users of `cs->jobs_in_queue_cnt` access it as an array, so the flexible array member works as a drop-in replacement.

**Issues:**

1. **Missing `@jobs_in_queue_cnt` doc comment update.** The kernel-doc comment for `struct hl_cs` at line 2007 describes `jobs_in_queue_cnt` as:
   ```
   * @jobs_in_queue_cnt: per each queue, maintain counter of submitted jobs.
   ```
   This remains valid, but since the member was moved from its original position (pointer at the top of the struct) to the end (as a FAM), the doc comment at `habanalabs.h:2007` should ideally be moved or noted as being at the end. Currently the doc comment ordering already has `jobs_in_queue_cnt` listed before `ctx` (line 2007 vs 2049), which matched the old layout but no longer matches the new layout. This is a minor nit — the comment content is still correct.

2. **Removed error path may hide a subtle change in behavior.** The original code had a separate `free_cs_cmpl` label in the error path:
   ```c
   free_fence:
       spin_unlock(&ctx->cs_lock);
       kfree(cs->jobs_in_queue_cnt);
   free_cs_cmpl:
       kfree(cs_cmpl);
   free_cs:
       kfree(cs);
   ```
   The patch removes the `free_cs_cmpl` label entirely, and the `free_fence` path now falls through directly to `kfree(cs_cmpl)`. This is correct because the `jobs_in_queue_cnt` allocation that previously jumped to `free_cs_cmpl` on failure has been eliminated. The remaining error paths (`free_fence` from the cs_lock section, and `free_cs` from cs_cmpl allocation failure) are properly handled. No issue here.

3. **No `__counted_by` annotation on the FAM.** The flexible array member is declared as:
   ```c
   u16			jobs_in_queue_cnt[];
   ```
   Modern kernel style recommends annotating FAMs with `__counted_by(field)` where feasible, to enable compile-time and runtime bounds checking. There doesn't appear to be a direct count field in the struct for this (the count comes from `hdev->asic_prop.max_queues`), so adding `__counted_by` would require adding a count field to the struct. This is not a blocker but could be a follow-up improvement.

4. **Struct layout / padding consideration.** The FAM `u16 jobs_in_queue_cnt[]` is placed after `u8 encaps_signals` at the end of the struct. Since the preceding field is `u8` and the FAM element type is `u16`, there will be 1 byte of padding inserted before the array. This is handled correctly by the compiler and `struct_size()` (which `kzalloc_flex` uses internally), so it's not a bug — just a minor observation that there's a wasted byte. The old layout with the pointer (`u16 *jobs_in_queue_cnt`) at the top of the struct actually occupied 8 bytes (pointer), so the overall savings are still positive.

**Verdict:** The patch is correct and a nice cleanup. It could use a `__counted_by` annotation on the FAM (possibly as a follow-up if a counter field is added to the struct), but is otherwise ready to go with minor doc comment ordering nit.

---
Generated by Claude Code Patch Reviewer