diff mbox series

[2/2] scsi: mpi3mr: Avoid MAX_PARE_ORDER WARNING for buffer allocations

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

Commit Message

Shinichiro Kawasaki Aug. 10, 2024, 4:27 a.m. UTC
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(-)

Comments

Sathya Prakash Veerichetty Aug. 12, 2024, 3:30 p.m. UTC | #1
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
>
Shinichiro Kawasaki Aug. 13, 2024, 12:23 a.m. UTC | #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.
Martin K. Petersen Aug. 13, 2024, 12:52 a.m. UTC | #3
Shinichiro,

>   s/MAX_PARE_ORDER/MAX_PAGE_ORDER/

I fixed it up, thanks!
diff mbox series

Patch

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",