* [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
@ 2026-04-20 12:17 Sasha Finkelstein
2026-04-20 13:30 ` Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sasha Finkelstein @ 2026-04-20 12:17 UTC (permalink / raw)
To: Aun-Ali Zaidi, Aditya Garg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, soopyc, 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>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
2026-04-20 12:17 [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Sasha Finkelstein
@ 2026-04-20 13:30 ` Thomas Zimmermann
2026-04-22 23:55 ` Claude review: " Claude Code Review Bot
2026-04-22 23:55 ` Claude Code Review Bot
2026-04-20 13:47 ` Thomas Zimmermann
2026-04-20 14:03 ` Aditya Garg
2 siblings, 2 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2026-04-20 13:30 UTC (permalink / raw)
To: Sasha Finkelstein, Aun-Ali Zaidi, Aditya Garg, Maarten Lankhorst,
Maxime Ripard, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, soopyc
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
2026-04-20 12:17 [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Sasha Finkelstein
2026-04-20 13:30 ` Thomas Zimmermann
@ 2026-04-20 13:47 ` Thomas Zimmermann
2026-04-20 14:03 ` Aditya Garg
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2026-04-20 13:47 UTC (permalink / raw)
To: Sasha Finkelstein, Aun-Ali Zaidi, Aditya Garg, Maarten Lankhorst,
Maxime Ripard, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, soopyc
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
2026-04-20 12:17 [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Sasha Finkelstein
2026-04-20 13:30 ` Thomas Zimmermann
2026-04-20 13:47 ` Thomas Zimmermann
@ 2026-04-20 14:03 ` Aditya Garg
2 siblings, 0 replies; 6+ messages in thread
From: Aditya Garg @ 2026-04-20 14:03 UTC (permalink / raw)
To: Sasha Finkelstein, Aun-Ali Zaidi, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, soopyc
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>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
2026-04-20 13:30 ` Thomas Zimmermann
@ 2026-04-22 23:55 ` Claude Code Review Bot
2026-04-22 23:55 ` Claude Code Review Bot
1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 23:55 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations
2026-04-20 13:30 ` Thomas Zimmermann
2026-04-22 23:55 ` Claude review: " Claude Code Review Bot
@ 2026-04-22 23:55 ` Claude Code Review Bot
1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 23:55 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-22 23:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 12:17 [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Sasha Finkelstein
2026-04-20 13:30 ` Thomas Zimmermann
2026-04-22 23:55 ` Claude review: " Claude Code Review Bot
2026-04-22 23:55 ` Claude Code Review Bot
2026-04-20 13:47 ` Thomas Zimmermann
2026-04-20 14:03 ` Aditya Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox