[PATCH] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports()

[PATCH] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports()

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Toubling the number of PHYs also doubled the stack usage of this function,
exceeding the 32-bit limit of 1024 bytes:

drivers/scsi/mpi3mr/mpi3mr_transport.c: In function 'mpi3mr_refresh_sas_ports':
drivers/scsi/mpi3mr/mpi3mr_transport.c:1818:1: error: the frame size of 1636 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Since the sas_io_unit_pg0 structure is already allocated dynamically, use
the same method here. The size of the allocation can be smaller based on the
actual number of phys now, so use this as an upper bound.

Fixes: cb5b60894602 ("scsi: mpi3mr: Increase maximum number of PHYs to 64 from 32")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index c0c8ab586957..ab04596dbdf5 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -1671,7 +1671,7 @@ mpi3mr_update_mr_sas_port(struct mpi3mr_ioc *mrioc, struct host_port *h_port,
 void
 mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 {
-	struct host_port h_port[64];
+	struct host_port *h_port = NULL;
 	int i, j, found, host_port_count = 0, port_idx;
 	u16 sz, attached_handle, ioc_status;
 	struct mpi3_sas_io_unit_page0 *sas_io_unit_pg0 = NULL;
@@ -1685,6 +1685,11 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 	sas_io_unit_pg0 = kzalloc(sz, GFP_KERNEL);
 	if (!sas_io_unit_pg0)
 		return;
+	h_port = kcalloc(mrioc->sas_hba.num_phys, sizeof(struct host_port),
+			 GFP_KERNEL);
+	if (!h_port)
+		goto out;
+
 	if (mpi3mr_cfg_get_sas_io_unit_pg0(mrioc, sas_io_unit_pg0, sz)) {
 		ioc_err(mrioc, "failure at %s:%d/%s()!\n",
 		    __FILE__, __LINE__, __func__);
@@ -1814,6 +1819,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		}
 	}
 out:
+	kfree(h_port);
 	kfree(sas_io_unit_pg0);
 }
 
-- 
2.39.2

Re: [PATCH] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports()

[Johannes Thumshirn]

On 29.11.23 13:06, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Toubling the number of PHYs also doubled the stack usage of this function,
   ^~ Doubling?

Otherwise looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports()

[Sathya Prakash Veerichetty]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]

On Wed, Nov 29, 2023 at 5:06 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Toubling the number of PHYs also doubled the stack usage of this function,
> exceeding the 32-bit limit of 1024 bytes:
>
> drivers/scsi/mpi3mr/mpi3mr_transport.c: In function 'mpi3mr_refresh_sas_ports':
> drivers/scsi/mpi3mr/mpi3mr_transport.c:1818:1: error: the frame size of 1636 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> Since the sas_io_unit_pg0 structure is already allocated dynamically, use
> the same method here. The size of the allocation can be smaller based on the
> actual number of phys now, so use this as an upper bound.
>
> Fixes: cb5b60894602 ("scsi: mpi3mr: Increase maximum number of PHYs to 64 from 32")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/scsi/mpi3mr/mpi3mr_transport.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> index c0c8ab586957..ab04596dbdf5 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> @@ -1671,7 +1671,7 @@ mpi3mr_update_mr_sas_port(struct mpi3mr_ioc *mrioc, struct host_port *h_port,
>  void
>  mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
>  {
> -       struct host_port h_port[64];
> +       struct host_port *h_port = NULL;
>         int i, j, found, host_port_count = 0, port_idx;
>         u16 sz, attached_handle, ioc_status;
>         struct mpi3_sas_io_unit_page0 *sas_io_unit_pg0 = NULL;
> @@ -1685,6 +1685,11 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
>         sas_io_unit_pg0 = kzalloc(sz, GFP_KERNEL);
>         if (!sas_io_unit_pg0)
>                 return;
> +       h_port = kcalloc(mrioc->sas_hba.num_phys, sizeof(struct host_port),
> +                        GFP_KERNEL);
>> We need more than sas_hba.num_phys, so can you please use 64 instead of sas_hba.num_phys. Otherwise looks good.
> +       if (!h_port)
> +               goto out;
> +
>         if (mpi3mr_cfg_get_sas_io_unit_pg0(mrioc, sas_io_unit_pg0, sz)) {
>                 ioc_err(mrioc, "failure at %s:%d/%s()!\n",
>                     __FILE__, __LINE__, __func__);
> @@ -1814,6 +1819,7 @@ mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
>                 }
>         }
>  out:
> +       kfree(h_port);
>         kfree(sas_io_unit_pg0);
>  }
>
> --
> 2.39.2
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4227 bytes --]