public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl
@ 2026-04-27 17:25 Thorsten Blum
  2026-04-27 17:40 ` T.J. Mercier
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thorsten Blum @ 2026-04-27 17:25 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König
  Cc: Thorsten Blum, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

Replace two nested max() calls with a single max3() in dma_heap_ioctl().

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/dma-buf/dma-heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index ac5f8685a649..52eec2ebb2e8 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -153,7 +153,7 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
 		in_size = 0;
 	if ((ucmd & kcmd & IOC_OUT) == 0)
 		out_size = 0;
-	ksize = max(max(in_size, out_size), drv_size);
+	ksize = max3(in_size, out_size, drv_size);
 
 	/* If necessary, allocate buffer for ioctl argument */
 	if (ksize > sizeof(stack_kdata)) {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl
  2026-04-27 17:25 [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl Thorsten Blum
@ 2026-04-27 17:40 ` T.J. Mercier
  2026-04-28  4:15 ` Claude review: " Claude Code Review Bot
  2026-04-28  4:15 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: T.J. Mercier @ 2026-04-27 17:40 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	Christian König, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

On Mon, Apr 27, 2026 at 10:26 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Replace two nested max() calls with a single max3() in dma_heap_ioctl().
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/dma-buf/dma-heap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index ac5f8685a649..52eec2ebb2e8 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -153,7 +153,7 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>                 in_size = 0;
>         if ((ucmd & kcmd & IOC_OUT) == 0)
>                 out_size = 0;
> -       ksize = max(max(in_size, out_size), drv_size);
> +       ksize = max3(in_size, out_size, drv_size);

Hi,

Could you add #include <linux/minmax.h> as well? It's currently missing.

With that: Reviewed-by: T.J.Mercier <tjmercier@google.com>

Thanks,
T.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: dma-buf: heaps: use max3() in dma_heap_ioctl
  2026-04-27 21:24 [PATCH v2] " Thorsten Blum
@ 2026-04-28  3:58 ` Claude Code Review Bot
  2026-04-28  3:58 ` Claude Code Review Bot
  1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  3:58 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dma-buf: heaps: use max3() in dma_heap_ioctl
Author: Thorsten Blum <thorsten.blum@linux.dev>
Patches: 1
Reviewed: 2026-04-28T13:58:20.743919

---

This is a single, trivial cleanup patch that replaces nested `max(max(a, b), c)` with the dedicated `max3(a, b, c)` macro. The change is correct, improves readability marginally, and has no functional impact. It already carries a Reviewed-by tag from a dma-buf maintainer (T.J. Mercier). No concerns.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: dma-buf: heaps: use max3() in dma_heap_ioctl
  2026-04-27 21:24 [PATCH v2] " Thorsten Blum
  2026-04-28  3:58 ` Claude review: " Claude Code Review Bot
@ 2026-04-28  3:58 ` Claude Code Review Bot
  1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  3:58 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Looks good**

The patch does two things:

1. Adds `#include <linux/minmax.h>` — correct, since `max3()` is defined there and was not previously included directly. Alphabetical ordering of the include is maintained.

2. Replaces:
   ```c
   ksize = max(max(in_size, out_size), drv_size);
   ```
   with:
   ```c
   ksize = max3(in_size, out_size, drv_size);
   ```

This is a straightforward substitution. `max3()` in `include/linux/minmax.h` (line 152) is defined via `__careful_op3(max, ...)` which performs the same type-safe three-way max. The semantics are identical.

**Minor observations:**
- The v1→v2 changelog is clear: v1 was missing the explicit `minmax.h` include, and T.J. Mercier caught it. Good iteration.
- `max()` is likely already transitively available (via `kernel.h` or similar), but adding the explicit `minmax.h` include for `max3()` is the right practice — it avoids relying on transitive includes that could break in future header cleanups.

No issues found. The patch is correct and ready to merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: dma-buf: heaps: use max3() in dma_heap_ioctl
  2026-04-27 17:25 [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl Thorsten Blum
  2026-04-27 17:40 ` T.J. Mercier
@ 2026-04-28  4:15 ` Claude Code Review Bot
  2026-04-28  4:15 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  4:15 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dma-buf: heaps: use max3() in dma_heap_ioctl
Author: Thorsten Blum <thorsten.blum@linux.dev>
Patches: 2
Reviewed: 2026-04-28T14:15:58.686759

---

This is a single-patch trivial cleanup that replaces nested `max(max(a, b), c)` with the equivalent `max3(a, b, c)` macro. The change is correct, improves readability marginally, and has no functional impact. The `max3` macro is already available through the existing include chain (via `linux/minmax.h`, included transitively through `linux/device.h` -> `linux/kernel.h`).

**Verdict:** Harmless cleanup. No concerns.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: dma-buf: heaps: use max3() in dma_heap_ioctl
  2026-04-27 17:25 [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl Thorsten Blum
  2026-04-27 17:40 ` T.J. Mercier
  2026-04-28  4:15 ` Claude review: " Claude Code Review Bot
@ 2026-04-28  4:15 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  4:15 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status:** Looks good.

The change replaces:
```c
ksize = max(max(in_size, out_size), drv_size);
```
with:
```c
ksize = max3(in_size, out_size, drv_size);
```

This is a straightforward substitution. `max3()` is defined in `include/linux/minmax.h:152` and is available in this compilation unit through the existing include chain. The three arguments (`in_size`, `out_size`, `drv_size`) are all the same type (`unsigned int` derived from the ioctl size encoding), so there are no type-promotion surprises.

The commit message is accurate and concise. The Signed-off-by is present.

**Reviewed-by worthy:** Yes, no issues found.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-28  4:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 17:25 [PATCH] dma-buf: heaps: use max3() in dma_heap_ioctl Thorsten Blum
2026-04-27 17:40 ` T.J. Mercier
2026-04-28  4:15 ` Claude review: " Claude Code Review Bot
2026-04-28  4:15 ` Claude Code Review Bot
  -- strict thread matches above, loose matches on Subject: below --
2026-04-27 21:24 [PATCH v2] " Thorsten Blum
2026-04-28  3:58 ` Claude review: " Claude Code Review Bot
2026-04-28  3:58 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox