[PATCH v2] drm/amdgpu: use bitmap_clear() in amdgpu_amdkfd_device_init()

[PATCH v2] drm/amdgpu: use bitmap_clear() in amdgpu_amdkfd_device_init()

[Yury Norov]

The bitmap_clear() works OK with both compile- and runtime nbits. But the
comment says it doesn't work, and opencodes the call for nothing.
Drop the misleading comment, and use bitmap_clear() as it should.

As a side effect, the patch switches from a series of atomics to
a single non-atomic operation, which is easier on caches.

Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
v2: don't declare 'i' in the new implementation.

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index d9e283f3b57d..500976d9087a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -167,7 +167,6 @@ int amdgpu_amdkfd_drm_client_create(struct amdgpu_device *adev)
 
 void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
 {
-	int i;
 	int last_valid_bit;
 
 	amdgpu_amdkfd_gpuvm_init_mem_limits();
@@ -194,14 +193,11 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
 				  adev->gfx.mec_bitmap[0].queue_bitmap,
 				  AMDGPU_MAX_QUEUES);
 
-		/* According to linux/bitmap.h we shouldn't use bitmap_clear if
-		 * nbits is not compile time constant
-		 */
 		last_valid_bit = 1 /* only first MEC can have compute queues */
 				* adev->gfx.mec.num_pipe_per_mec
 				* adev->gfx.mec.num_queue_per_pipe;
-		for (i = last_valid_bit; i < AMDGPU_MAX_QUEUES; ++i)
-			clear_bit(i, gpu_resources.cp_queue_bitmap);
+		bitmap_clear(gpu_resources.cp_queue_bitmap, last_valid_bit,
+					AMDGPU_MAX_QUEUES - last_valid_bit);
 
 		amdgpu_doorbell_get_kfd_info(adev,
 				&gpu_resources.doorbell_physical_address,
-- 
2.51.0

Claude review: drm/amdgpu: use bitmap_clear() in amdgpu_amdkfd_device_init()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amdgpu: use bitmap_clear() in amdgpu_amdkfd_device_init()
Author: Yury Norov <ynorov@nvidia.com>
Patches: 1
Reviewed: 2026-04-28T13:43:04.392665

---

This is a single-patch cleanup series from Yury Norov that replaces a hand-rolled `clear_bit()` loop with `bitmap_clear()` in the amdgpu KFD device init path. The patch is correct and straightforward.

The original code contained a misleading comment claiming `bitmap_clear()` shouldn't be used when `nbits` is not a compile-time constant. Looking at the actual `bitmap_clear()` implementation in `include/linux/bitmap.h:547`, it's clear this claim was wrong — the function has an `else` fallback to `__bitmap_clear()` that handles runtime-variable `nbits` just fine. The compile-time-constant paths (single-bit, small_const_nbits, aligned memset) are just optimizations, not requirements.

The change is safe, correct, and provides a minor performance improvement by replacing N individual atomic `clear_bit()` calls with a single non-atomic `bitmap_clear()`, which operates on whole words at a time.

**Verdict: Looks good to merge.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amdgpu: use bitmap_clear() in amdgpu_amdkfd_device_init()

[Claude Code Review Bot]

Patch Review

**Correctness:**

The transformation from:
```c
for (i = last_valid_bit; i < AMDGPU_MAX_QUEUES; ++i)
    clear_bit(i, gpu_resources.cp_queue_bitmap);
```
to:
```c
bitmap_clear(gpu_resources.cp_queue_bitmap, last_valid_bit,
             AMDGPU_MAX_QUEUES - last_valid_bit);
```
is semantically correct. Both clear the same range of bits `[last_valid_bit, AMDGPU_MAX_QUEUES)`.

**Atomicity change is safe:** The old code used `clear_bit()` (atomic), while `bitmap_clear()` uses `__bitmap_clear()` (non-atomic) for the runtime-nbits path. This is fine because `gpu_resources` is a local variable on the stack — it was just initialized a few lines above and no other thread can observe it. There is no need for atomic operations on stack-local data.

**Removal of misleading comment:** Correct to remove. The comment:
```c
/* According to linux/bitmap.h we shouldn't use bitmap_clear if
 * nbits is not compile time constant
 */
```
was wrong. `bitmap_clear()` at `include/linux/bitmap.h:547` handles non-constant `nbits` via `__bitmap_clear()`.

**Removal of `int i`:** Clean — the loop variable is no longer needed, and `last_valid_bit` is the only remaining local.

**Alignment nit (cosmetic only):** The continuation line aligns `AMDGPU_MAX_QUEUES - last_valid_bit` with a tab+spaces to match the opening paren, which results in slightly unusual indentation but is consistent with the surrounding code style (e.g., the `bitmap_complement` call just above uses the same pattern).

**No functional concerns.** This is a clean, correct simplification.

**Reviewed-by recommendation: Yes.**

---
Generated by Claude Code Patch Reviewer