Skip to contentSkip to conversation

scsi: sd_zbc: use kvzalloc to allocate report zones buffer

PATCH In review Johannes Thumshirn 4 participants 5 messages Oct 30, 2024 – Nov 8, 2024 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 scsi: sd_zbc: use kvzalloc to allocate report zones buffer

From: Johannes Thumshirn <[email protected]>

We have two reports of failed memory allocation in btrfs' code which is calling into report zones.

Both of these reports have the following signature coming from __vmalloc_area_node():

kworker/u17:5: vmalloc error: size 0, failed to allocate pages, mode:0x10dc2(GFP_KERNEL|__GFP_HIGHMEM|__GFP_NORETRY|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0

Further debugging showed these where allocations of one sector (512 bytes) and at least one of the reporter's systems where low on memory, so going through the overhead of allocating a vm area failed.

Switching the allocation from __vmalloc() to kvzalloc() avoids the overhead of vmalloc() on small allocations and succeeds.

Note: the buffer is already freed using kvfree() so there's no need to adjust the free path.

Cc Qu Wenru <[email protected]>
Cc Naohiro Aota <[email protected]>
Fixes Fixes: 23a50861adda ("scsi: sd_zbc: Cleanup sd_zbc_alloc_report_buffer()")

---

Note2: one of the reporters tested the patch with kvmalloc(... , ... | __GFP_ZERO) instead of kvzalloc(). This is an "optimization" I did after the successful testing.

Note3: calling report zones every time we create a block group in btrfs is suboptimal as well and we're going to change this code path as well ASAP.

---
 drivers/scsi/sd_zbc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/sd_zbc.c +1 -2
index ee2b74238758..6ab27f4f4878 100644
@@ -188,8 +188,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
 	bufsize = min_t(size_t, bufsize, queue_max_segments(q) << PAGE_SHIFT);
 
 	while (bufsize >= SECTOR_SIZE) {
-		buf = __vmalloc(bufsize,
-				GFP_KERNEL | __GFP_ZERO | __GFP_NORETRY);
+		buf = kvzalloc(bufsize, GFP_KERNEL | __GFP_NORETRY);
 		if (buf) {
 			*buflen = bufsize;
 			return buf;
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 20 lines · Johannes Thumshirn

From: Johannes Thumshirn <[email protected]>

We have two reports of failed memory allocation in btrfs' code which is calling into report zones.

Both of these reports have the following signature coming from __vmalloc_area_node():

kworker/u17:5: vmalloc error: size 0, failed to allocate pages, mode:0x10dc2(GFP_KERNEL|__GFP_HIGHMEM|__GFP_NORETRY|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0

Further debugging showed these where allocations of one sector (512 bytes) and at least one of the reporter's systems where low on memory, so going through the overhead of allocating a vm area failed.

Switching the allocation from __vmalloc() to kvzalloc() avoids the overhead of vmalloc() on small allocations and succeeds.

Note: the buffer is already freed using kvfree() so there's no need to adjust the free path.

Quoted 6 lines
Cc Qu Wenru <[email protected]>
Cc Naohiro Aota <[email protected]>
Fixes Fixes: 23a50861adda ("scsi: sd_zbc: Cleanup sd_zbc_alloc_report_buffer()")

Looks good.

Signature · 2 lines
Damien Le Moal
Western Digital Research
#4 lore raw
Quoted 12 lines · Martin K. Petersen

Hi Johannes!

Quoted 1 line

ping?

It's already in scsi-fixes:

   https://git.kernel.org/mkp/scsi/c/7ce3e6107103

I don't see a corresponding merge notification mail in lore but the patch is marked Accepted in patchwork. b4 gets confused sometimes...

Ah sorry, I didn't check the brach, just waited for the email.