| Message ID | 20240810042701.661841-3-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: mpi3mr: Fix two bugs in the new buffer allocation code | expand |
On Fri, Aug 9, 2024 at 10:27 PM Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> wrote: > > Commit fc4444941140 ("scsi: mpi3mr: HDB allocation and posting for > hardware and firmware buffers") added mpi3mr_alloc_diag_bufs() which > calls dma_alloc_coherent() to allocate the trace buffer and the firmware > buffer. mpi3mr_alloc_diag_bufs() decides the buffer sizes from the > driver configuration. In my environment, the sizes are 8MB. With the > sizes, dma_alloc_coherent() fails and report this WARNING: > > WARNING: CPU: 4 PID: 438 at mm/page_alloc.c:4676 __alloc_pages_noprof+0x52f/0x640 > > The WARNING indicates that the order of the allocation size is larger > than MAX_PAGE_ORDER. After this failure, mpi3mr_alloc_diag_bufs() > reduces the buffer sizes and retries dma_alloc_coherent(). In the end, > the buffer allocations succeed with 4MB size in my environment, which > corresponds to MAX_PAGE_ORDER=10. Though the allocations succeed, the > WARNING message is misleading and should be avoided. > > To avoid the WARNING, check the orders of the buffer allocation sizes > before calling dma_alloc_coherent(). If the orders are larger than > MAX_PAGE_ORDER, fall back to the retry path. > > Fixes: fc4444941140 ("scsi: mpi3mr: HDB allocation and posting for hardware and firmware buffers") > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- Acked-by: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com> > drivers/scsi/mpi3mr/mpi3mr_app.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c > index 8b0eded6ef36..01f035f9330e 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_app.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c > @@ -100,7 +100,8 @@ void mpi3mr_alloc_diag_bufs(struct mpi3mr_ioc *mrioc) > dprint_init(mrioc, > "trying to allocate trace diag buffer of size = %dKB\n", > trace_size / 1024); > - if (mpi3mr_alloc_trace_buffer(mrioc, trace_size)) { > + if (get_order(trace_size) > MAX_PAGE_ORDER || > + mpi3mr_alloc_trace_buffer(mrioc, trace_size)) { > retry = true; > trace_size -= trace_dec_size; > dprint_init(mrioc, "trace diag buffer allocation failed\n" > @@ -118,8 +119,12 @@ void mpi3mr_alloc_diag_bufs(struct mpi3mr_ioc *mrioc) > diag_buffer->type = MPI3_DIAG_BUFFER_TYPE_FW; > diag_buffer->status = MPI3MR_HDB_BUFSTATUS_NOT_ALLOCATED; > if ((mrioc->facts.diag_fw_sz < fw_size) && (fw_size >= fw_min_size)) { > - diag_buffer->addr = dma_alloc_coherent(&mrioc->pdev->dev, > - fw_size, &diag_buffer->dma_addr, GFP_KERNEL); > + if (get_order(fw_size) <= MAX_PAGE_ORDER) { > + diag_buffer->addr > + = dma_alloc_coherent(&mrioc->pdev->dev, fw_size, > + &diag_buffer->dma_addr, > + GFP_KERNEL); > + } > if (!retry) > dprint_init(mrioc, > "%s:trying to allocate firmware diag buffer of size = %dKB\n", > -- > 2.45.2 >
I made a spell mistake in the commit title: s/MAX_PARE_ORDER/MAX_PAGE_ORDER/ Martin, if this patch gets applied as it is, could you fix the mistake? If you want me to repost with the fix, please let me know.
Shinichiro,
> s/MAX_PARE_ORDER/MAX_PAGE_ORDER/
I fixed it up, thanks!
diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c index 8b0eded6ef36..01f035f9330e 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_app.c +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c @@ -100,7 +100,8 @@ void mpi3mr_alloc_diag_bufs(struct mpi3mr_ioc *mrioc) dprint_init(mrioc, "trying to allocate trace diag buffer of size = %dKB\n", trace_size / 1024); - if (mpi3mr_alloc_trace_buffer(mrioc, trace_size)) { + if (get_order(trace_size) > MAX_PAGE_ORDER || + mpi3mr_alloc_trace_buffer(mrioc, trace_size)) { retry = true; trace_size -= trace_dec_size; dprint_init(mrioc, "trace diag buffer allocation failed\n" @@ -118,8 +119,12 @@ void mpi3mr_alloc_diag_bufs(struct mpi3mr_ioc *mrioc) diag_buffer->type = MPI3_DIAG_BUFFER_TYPE_FW; diag_buffer->status = MPI3MR_HDB_BUFSTATUS_NOT_ALLOCATED; if ((mrioc->facts.diag_fw_sz < fw_size) && (fw_size >= fw_min_size)) { - diag_buffer->addr = dma_alloc_coherent(&mrioc->pdev->dev, - fw_size, &diag_buffer->dma_addr, GFP_KERNEL); + if (get_order(fw_size) <= MAX_PAGE_ORDER) { + diag_buffer->addr + = dma_alloc_coherent(&mrioc->pdev->dev, fw_size, + &diag_buffer->dma_addr, + GFP_KERNEL); + } if (!retry) dprint_init(mrioc, "%s:trying to allocate firmware diag buffer of size = %dKB\n",
Commit fc4444941140 ("scsi: mpi3mr: HDB allocation and posting for hardware and firmware buffers") added mpi3mr_alloc_diag_bufs() which calls dma_alloc_coherent() to allocate the trace buffer and the firmware buffer. mpi3mr_alloc_diag_bufs() decides the buffer sizes from the driver configuration. In my environment, the sizes are 8MB. With the sizes, dma_alloc_coherent() fails and report this WARNING: WARNING: CPU: 4 PID: 438 at mm/page_alloc.c:4676 __alloc_pages_noprof+0x52f/0x640 The WARNING indicates that the order of the allocation size is larger than MAX_PAGE_ORDER. After this failure, mpi3mr_alloc_diag_bufs() reduces the buffer sizes and retries dma_alloc_coherent(). In the end, the buffer allocations succeed with 4MB size in my environment, which corresponds to MAX_PAGE_ORDER=10. Though the allocations succeed, the WARNING message is misleading and should be avoided. To avoid the WARNING, check the orders of the buffer allocation sizes before calling dma_alloc_coherent(). If the orders are larger than MAX_PAGE_ORDER, fall back to the retry path. Fixes: fc4444941140 ("scsi: mpi3mr: HDB allocation and posting for hardware and firmware buffers") Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- drivers/scsi/mpi3mr/mpi3mr_app.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)