Re: [PATCH 3/4] scsi: ipr: use kmalloc() to allocate IPR dump buffer memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Switching from __get_free_page() to kmalloc() for bulk PAGE_SIZE allocations worsens memory fragmentation resilience and increases the likelihood of allocation failure.

Pre-existing issues:
- [High] Unnecessary use of GFP_ATOMIC in a fully sleepable context drains atomic memory reserves and causes large allocations to fail.
--

commit 031dbb9ad5b7614bab879909db8f301be9752516
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

scsi: ipr: use kmalloc() to allocate IPR dump buffer memory

This commit replaces __get_free_page() and free_page() with kmalloc() and
kfree() for allocating the IPR dump buffer. The goal is to provide a cleaner
API without requiring casts or tracking the allocation size during free.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index d207e5e81afe0..5a212bfdeec2b 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -2893,7 +2893,7 @@ static int ipr_sdt_copy(struct ipr_ioa_cfg *ioa_cfg,
>  	       (ioa_dump->hdr.len + bytes_copied) < max_dump_size) {
>  		if (ioa_dump->page_offset >= PAGE_SIZE ||
>  		    ioa_dump->page_offset == 0) {
> -			page = (__be32 *)__get_free_page(GFP_ATOMIC);
> +			page = kmalloc(PAGE_SIZE, GFP_ATOMIC);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but does this allocation actually need to use
GFP_ATOMIC?

Looking at ipr_sdt_copy(), the allocation happens in a loop that explicitly
yields the CPU by calling schedule() at the end of each iteration. No spinlocks
are held across the allocation (host_lock is acquired later). Since the context
appears to be fully sleepable, using GFP_ATOMIC to allocate up to 80MB of memory
unnecessarily drains emergency reserves, potentially starving actual atomic
contexts like interrupt handlers.

[Severity: High]
Does switching to kmalloc() for PAGE_SIZE chunks increase the likelihood of
allocation failures under memory fragmentation?

If memory debugging options like KASAN or SLUB debugging are enabled, the SLUB
allocator inflates the requested PAGE_SIZE with metadata. This pushes the
allocation into a larger size class (e.g., kmalloc-8k), requiring higher-order
page allocations (order-1 or more) from the page allocator. Since the
allocation still uses GFP_ATOMIC, which cannot sleep or wait for direct reclaim,
requesting order-1+ pages is far more likely to fail under memory fragmentation
compared to the original order-0 requests.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-b4-scsi-v1-0-494fb37ebe7b@kernel.org?part=3