Re: [PATCH] firmware: cs_dsp: Fix fragmentation regression in firmware download

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 04/03/2026 3:17 pm, Takashi Iwai wrote:
> On Wed, 04 Mar 2026 15:12:50 +0100,
> Richard Fitzgerald wrote:
>>
>> Use vmalloc() instead of kmalloc(..., GFP_DMA) to alloc the temporary
>> buffer for firmware download blobs. This avoids the problem that a
>> heavily fragmented system cannot allocate enough physically-contiguous
>> memory for a large blob.
>>
>> The redundant alloc buffer mechanism was removed in commit 900baa6e7bb0
>> ("firmware: cs_dsp: Remove redundant download buffer allocator").
>> While doing that I was overly focused on the possibility of the
>> underlying bus requiring DMA-safe memory. So I used GFP_DMA kmalloc()s.
>> I failed to notice that the code I was removing used vmalloc().
>> This creates a regression.
>>
>> Way back in 2014 the problem of fragmentation with kmalloc()s was fixed
>> by commit cdcd7f728753 ("ASoC: wm_adsp: Use vmalloc to allocate firmware
>> download buffer").
>>
>> Although we don't need physically-contiguous memory, we don't know if the
>> bus needs some particular alignment of the buffers. Since the change in
>> 2014, the firmware download has always used whatever alignment vmalloc()
>> returns. To avoid introducing a new problem, the temporary buffer is still
>> used, to keep the same alignment of pointers passed to regmap_raw_write().
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> Fixes: 900baa6e7bb0 ("firmware: cs_dsp: Remove redundant download buffer allocator")
> 
> FYI, if the data isn't always large, kvmalloc() could be a better
> alternative, which is a hybrid for both speed and size, too.
> 
> 
> Takashi

I originally did that, but as this is a bugfix for backporting I decided
not to risk introducing a change inside a bugfix. The original code was
vmalloc() so I have corrected back to what the original code did.

vmalloc() appears to allocate whole pages, on PAGE_SIZE boundary but
kvmalloc() could allocate on a smaller boundary. I know that kmalloc()
memory is claimed to be be DMA-safe. But I don't want to risk fixing one
regression and introducing a new regression into stable kernels.

It's on my to-do list to have a look at using kvmalloc(), and possibly
skipping the buffer if the source data is already aligned on
ARCH_KMALLOC_MINALIGN. But that's for future kernel releases.