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

On Wed, Jul 01, 2026 at 09:03:06AM +0200, Hannes Reinecke wrote:
> On 6/30/26 12:54 PM, Mike Rapoport (Microsoft) wrote:
> > IPR dump machinery allocates memory to save adapter's crash dump using
> > __get_free_page().
> > 
> > This memory can be allocated with kmalloc() as there's nothing special
> > about it to go directly to the page allocator.
> > 
> > kmalloc() provides a better API that does not require ugly casts and
> > kfree() does not need to know the size of the freed object.
> > 
> > Replace use of __get_free_page() with kmalloc().
> > 
> > Link: https://lore.kernel.org/all/[email protected]
> > Signed-off-by: Mike Rapoport (Microsoft) <[email protected]>
> > ---
> >   drivers/scsi/ipr.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> > index d207e5e81afe..5a212bfdeec2 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);
> >   			if (!page) {
> >   				ipr_trace;
> > @@ -3226,7 +3226,7 @@ static void ipr_release_dump(struct kref *kref)
> >   	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> >   	for (i = 0; i < dump->ioa_dump.next_page_index; i++)
> > -		free_page((unsigned long) dump->ioa_dump.ioa_data[i]);
> > +		kfree(dump->ioa_dump.ioa_data[i]);
> >   	vfree(dump->ioa_dump.ioa_data);
> >   	kfree(dump);
> > 
>
> I _think_ we can replace this with kvmalloc, and allocate the entire
> dump buffer in one go. Once switched to kmalloc() it's kinda pointless
> to allocate separate page-sized buffers here.

kmalloc() performance is on par with __get_free_page(), but kvmalloc()
would be slower if it falls back to vmalloc(). 

I'm not familiar with the driver to say if this could be an issue here.
 
> Cheers,
> Hannes

-- 
Sincerely yours,
Mike.