diff mbox series

scsi: sd_zbc: use kvzalloc to allocate report zones buffer

Message ID 20241030110253.11718-1-jth@kernel.org
State New
Headers show
Series scsi: sd_zbc: use kvzalloc to allocate report zones buffer | expand

Commit Message

Johannes Thumshirn Oct. 30, 2024, 11:02 a.m. UTC 
From: Johannes Thumshirn <johannes.thumshirn@wdc.com>

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 <wqu@suse.com>
Cc: Naohiro Aota <naohiro.aota@wdc.com>
Link: https://github.com/kdave/btrfs-progs/issues/779
Link: https://github.com/kdave/btrfs-progs/issues/915
Fixes: Fixes: 23a50861adda ("scsi: sd_zbc: Cleanup sd_zbc_alloc_report_buffer()")
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---

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(-)

Comments

Damien Le Moal Oct. 30, 2024, 11:49 p.m. UTC | #1 
On 10/30/24 8:02 PM, Johannes Thumshirn wrote:
> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> 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 <wqu@suse.com>
> Cc: Naohiro Aota <naohiro.aota@wdc.com>
> Link: https://github.com/kdave/btrfs-progs/issues/779
> Link: https://github.com/kdave/btrfs-progs/issues/915
> Fixes: Fixes: 23a50861adda ("scsi: sd_zbc: Cleanup sd_zbc_alloc_report_buffer()")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Johannes Thumshirn Nov. 7, 2024, 8:46 a.m. UTC | #2 
Martin,

ping?
Martin K. Petersen Nov. 7, 2024, 9:01 p.m. UTC | #3 
Hi Johannes!

> 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...
Johannes Thumshirn Nov. 8, 2024, 7 a.m. UTC | #4 
On 07.11.24 22:02, Martin K. Petersen wrote:
> 
> Hi Johannes!
> 
>> 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.
diff mbox series

Patch

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index ee2b74238758..6ab27f4f4878 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -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;