[PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Sasha Finkelstein]

This driver is attached to a ~2000x80 screen, which is a lot more than
a single page. This causes out of memory errors in some rare cases.

Reported-by: soopyc <cassie@soopy.moe>
Closes: https://github.com/t2linux/fedora/issues/51
Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
---
 drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
index 3bae91d7eefe..278bb23fe4c8 100644
--- a/drivers/gpu/drm/tiny/appletbdrm.c
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -353,7 +353,7 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
 		       frames_size +
 		       sizeof(struct appletbdrm_fb_request_footer), 16);
 
-	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
+	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);
 
 	if (!appletbdrm_state->request)
 		return -ENOMEM;
@@ -543,7 +543,7 @@ static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
 {
 	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
 
-	kfree(appletbdrm_state->request);
+	kvfree(appletbdrm_state->request);
 	kfree(appletbdrm_state->response);
 
 	__drm_gem_destroy_shadow_plane_state(&appletbdrm_state->base);

---
base-commit: c1f49dea2b8f335813d3b348fd39117fb8efb428
change-id: 20260420-x86-tb-vmalloc-4c94d5eaeeae

Best regards,
--  
Sasha Finkelstein <k@chaosmail.tech>

Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Thomas Zimmermann]

Am 20.04.26 um 14:17 schrieb Sasha Finkelstein:
> This driver is attached to a ~2000x80 screen, which is a lot more than
> a single page. This causes out of memory errors in some rare cases.
>
> Reported-by: soopyc <cassie@soopy.moe>
> Closes: https://github.com/t2linux/fedora/issues/51
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index 3bae91d7eefe..278bb23fe4c8 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
> @@ -353,7 +353,7 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   		       frames_size +
>   		       sizeof(struct appletbdrm_fb_request_footer), 16);
>   
> -	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
> +	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);
>   
>   	if (!appletbdrm_state->request)
>   		return -ENOMEM;
> @@ -543,7 +543,7 @@ static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
>   {
>   	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
>   
> -	kfree(appletbdrm_state->request);
> +	kvfree(appletbdrm_state->request);
>   	kfree(appletbdrm_state->response);
>   
>   	__drm_gem_destroy_shadow_plane_state(&appletbdrm_state->base);
>
> ---
> base-commit: c1f49dea2b8f335813d3b348fd39117fb8efb428
> change-id: 20260420-x86-tb-vmalloc-4c94d5eaeeae
>
> Best regards,
> --
> Sasha Finkelstein <k@chaosmail.tech>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Claude review: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Claude Code Review Bot]

Patch Review

**NAK — vmalloc memory cannot be used as a USB DMA transfer buffer.**

The allocation site:
```c
-	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
+	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);
```

This buffer is subsequently passed to USB bulk transfer at `appletbdrm.c:449`:
```c
ret = appletbdrm_send_request(adev, &request->header, request_size);
```

Which calls `usb_bulk_msg()` at line 160:
```c
ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, adev->out_ep),
		   request, size, &actual_size, APPLETBDRM_BULK_MSG_TIMEOUT);
```

The USB HCD layer DMA-maps this buffer via `dma_map_single()` (`drivers/usb/core/hcd.c:1488`), and the DMA subsystem explicitly rejects vmalloc addresses (`include/linux/dma-mapping.h:513-515`). When `kvzalloc` falls back to vmalloc under memory pressure — the exact scenario this patch targets — the USB transfer will fail.

The free-side change is correct in isolation (matching `kvzalloc` with `kvfree`), but is moot given the allocation change is wrong:
```c
-	kfree(appletbdrm_state->request);
+	kvfree(appletbdrm_state->request);
```

**Suggestion:** Replace `kvzalloc` with `kzalloc` using `__GFP_RETRY_MAYFAIL`:
```c
appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
```

This tells the page allocator to try harder (compacting memory, reclaiming pages) before giving up, while keeping the allocation physically contiguous and safe for DMA. No change needed on the free side since `kfree` remains correct.

---
Generated by Claude Code Patch Reviewer

Claude review: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Claude Code Review Bot]

Overall Series Review

Subject: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 4
Reviewed: 2026-04-23T09:55:24.383215

---

This is a single patch that attempts to fix rare OOM failures in the appletbdrm driver by switching the frame request buffer from `kzalloc`/`kfree` to `kvzalloc`/`kvfree`. The motivation is sound — the Apple Touch Bar is ~2000x80 pixels, so a full damage update produces a request buffer on the order of 500-640 KB (depending on format), which requires a high-order contiguous allocation that can fail under memory fragmentation.

**However, the patch introduces a critical correctness bug.** The allocated buffer is passed directly to `usb_bulk_msg()`, which in turn calls `dma_map_single()` on the buffer via the USB HCD DMA mapping path. The kernel's DMA mapping layer **explicitly rejects vmalloc addresses**:

```c
/* include/linux/dma-mapping.h:512-515 */
/* DMA must never operate on areas that might be remapped. */
if (dev_WARN_ONCE(dev, is_vmalloc_addr(ptr),
		  "rejecting DMA map of vmalloc memory\n"))
	return DMA_MAPPING_ERROR;
```

If `kvzalloc` falls back to `vmalloc` (which is the entire point of the change), passing that buffer to `usb_bulk_msg()` will trigger a `WARN_ONCE`, the DMA mapping will fail with `DMA_MAPPING_ERROR`, and the USB transfer will return `-EAGAIN`. This would make the display non-functional under exactly the memory pressure conditions the patch is meant to fix.

**Recommended alternatives:**

1. **`kzalloc(request_size, GFP_KERNEL | __GFP_RETRY_MAYFAIL)`** — instructs the allocator to try harder (compaction, reclaim) without the hard failure semantics, while keeping the allocation physically contiguous and DMA-safe. This is the simplest correct fix.
2. **Restructure to use scatterlist-based USB transfers** — allows fragmented physical pages, but requires significant refactoring.
3. **Use `usb_alloc_coherent()`** — provides a DMA-safe buffer, but changes the allocation/free API.

---

---
Generated by Claude Code Patch Reviewer

Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Thomas Zimmermann]

Am 20.04.26 um 14:17 schrieb Sasha Finkelstein:
> This driver is attached to a ~2000x80 screen, which is a lot more than
> a single page. This causes out of memory errors in some rare cases.
>
> Reported-by: soopyc <cassie@soopy.moe>
> Closes: https://github.com/t2linux/fedora/issues/51
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>

Fixes: 0670c2f56e45 ("drm/tiny: add driver for Apple Touch Bars in x86 
Macs")
Cc: <stable@vger.kernel.org> # v6.15+


> ---
>   drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index 3bae91d7eefe..278bb23fe4c8 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
> @@ -353,7 +353,7 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   		       frames_size +
>   		       sizeof(struct appletbdrm_fb_request_footer), 16);
>   
> -	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
> +	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);
>   
>   	if (!appletbdrm_state->request)
>   		return -ENOMEM;
> @@ -543,7 +543,7 @@ static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
>   {
>   	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
>   
> -	kfree(appletbdrm_state->request);
> +	kvfree(appletbdrm_state->request);
>   	kfree(appletbdrm_state->response);
>   
>   	__drm_gem_destroy_shadow_plane_state(&appletbdrm_state->base);
>
> ---
> base-commit: c1f49dea2b8f335813d3b348fd39117fb8efb428
> change-id: 20260420-x86-tb-vmalloc-4c94d5eaeeae
>
> Best regards,
> --
> Sasha Finkelstein <k@chaosmail.tech>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations

[Aditya Garg]

Thanks for the fix!

Reviewed-by: Aditya Garg <gargaditya08@live.com>

On 4/20/26 17:47, Sasha Finkelstein wrote:
> This driver is attached to a ~2000x80 screen, which is a lot more than
> a single page. This causes out of memory errors in some rare cases.
> 
> Reported-by: soopyc <cassie@soopy.moe>
> Closes: https://github.com/t2linux/fedora/issues/51
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---
>   drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index 3bae91d7eefe..278bb23fe4c8 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
> @@ -353,7 +353,7 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   		       frames_size +
>   		       sizeof(struct appletbdrm_fb_request_footer), 16);
>   
> -	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
> +	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);
>   
>   	if (!appletbdrm_state->request)
>   		return -ENOMEM;
> @@ -543,7 +543,7 @@ static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
>   {
>   	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
>   
> -	kfree(appletbdrm_state->request);
> +	kvfree(appletbdrm_state->request);
>   	kfree(appletbdrm_state->response);
>   
>   	__drm_gem_destroy_shadow_plane_state(&appletbdrm_state->base);
> 
> ---
> base-commit: c1f49dea2b8f335813d3b348fd39117fb8efb428
> change-id: 20260420-x86-tb-vmalloc-4c94d5eaeeae
> 
> Best regards,
> --
> Sasha Finkelstein <k@chaosmail.tech>
>