From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Date: Thu, 23 Apr 2026 09:55:24 +1000 Message-ID: In-Reply-To: <302f22e3-5390-4c0a-aa06-bfd02a3175c0@suse.de> References: <302f22e3-5390-4c0a-aa06-bfd02a3175c0@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: Re: [PATCH] drm/appletbdrm: Use kvzalloc for big allocations Author: Thomas Zimmermann Patches: 4 Reviewed: 2026-04-23T09:55:24.383215 --- This is a single patch that attempts to fix rare OOM failures in the applet= bdrm driver by switching the frame request buffer from `kzalloc`/`kfree` to= `kvzalloc`/`kvfree`. The motivation is sound =E2=80=94 the Apple Touch Bar= is ~2000x80 pixels, so a full damage update produces a request buffer on t= he order of 500-640 KB (depending on format), which requires a high-order c= ontiguous 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_ma= p_single()` on the buffer via the USB HCD DMA mapping path. The kernel's DM= A 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 cha= nge), passing that buffer to `usb_bulk_msg()` will trigger a `WARN_ONCE`, t= he DMA mapping will fail with `DMA_MAPPING_ERROR`, and the USB transfer wil= l return `-EAGAIN`. This would make the display non-functional under exactl= y the memory pressure conditions the patch is meant to fix. **Recommended alternatives:** 1. **`kzalloc(request_size, GFP_KERNEL | __GFP_RETRY_MAYFAIL)`** =E2=80=94 = instructs the allocator to try harder (compaction, reclaim) without the har= d 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** =E2=80=94 allows = fragmented physical pages, but requires significant refactoring. 3. **Use `usb_alloc_coherent()`** =E2=80=94 provides a DMA-safe buffer, but= changes the allocation/free API. --- --- Generated by Claude Code Patch Reviewer