Skip to content

Commit facfdef

Browse files
rfvirgilbroonie
authored andcommitted
firmware: cs_dsp: Fix fragmentation regression in firmware download
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 900baa6 ("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 cdcd7f7 ("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: 900baa6 ("firmware: cs_dsp: Remove redundant download buffer allocator") Link: https://patch.msgid.link/20260304141250.1578597-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
1 parent d6db827 commit facfdef

1 file changed

Lines changed: 18 additions & 6 deletions

File tree

drivers/firmware/cirrus/cs_dsp.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,11 +1610,17 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
16101610
region_name);
16111611

16121612
if (reg) {
1613+
/*
1614+
* Although we expect the underlying bus does not require
1615+
* physically-contiguous buffers, we pessimistically use
1616+
* a temporary buffer instead of trusting that the
1617+
* alignment of region->data is ok.
1618+
*/
16131619
region_len = le32_to_cpu(region->len);
16141620
if (region_len > buf_len) {
16151621
buf_len = round_up(region_len, PAGE_SIZE);
1616-
kfree(buf);
1617-
buf = kmalloc(buf_len, GFP_KERNEL | GFP_DMA);
1622+
vfree(buf);
1623+
buf = vmalloc(buf_len);
16181624
if (!buf) {
16191625
ret = -ENOMEM;
16201626
goto out_fw;
@@ -1643,7 +1649,7 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
16431649

16441650
ret = 0;
16451651
out_fw:
1646-
kfree(buf);
1652+
vfree(buf);
16471653

16481654
if (ret == -EOVERFLOW)
16491655
cs_dsp_err(dsp, "%s: file content overflows file data\n", file);
@@ -2331,11 +2337,17 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
23312337
}
23322338

23332339
if (reg) {
2340+
/*
2341+
* Although we expect the underlying bus does not require
2342+
* physically-contiguous buffers, we pessimistically use
2343+
* a temporary buffer instead of trusting that the
2344+
* alignment of blk->data is ok.
2345+
*/
23342346
region_len = le32_to_cpu(blk->len);
23352347
if (region_len > buf_len) {
23362348
buf_len = round_up(region_len, PAGE_SIZE);
2337-
kfree(buf);
2338-
buf = kmalloc(buf_len, GFP_KERNEL | GFP_DMA);
2349+
vfree(buf);
2350+
buf = vmalloc(buf_len);
23392351
if (!buf) {
23402352
ret = -ENOMEM;
23412353
goto out_fw;
@@ -2366,7 +2378,7 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
23662378

23672379
ret = 0;
23682380
out_fw:
2369-
kfree(buf);
2381+
vfree(buf);
23702382

23712383
if (ret == -EOVERFLOW)
23722384
cs_dsp_err(dsp, "%s: file content overflows file data\n", file);

0 commit comments

Comments
 (0)