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

Message ID 20240123130754.2011469-1-arnd@kernel.org
State New
Headers
Series [v2] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports() |

Commit Message

Arnd Bergmann Jan. 23, 2024, 1:07 p.m. UTC
  From: Arnd Bergmann <arnd@arndb.de>

Doubling 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")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix number of ports to be always 64 rather than num_phys
---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Comments

John Garry Feb. 19, 2024, 12:08 p.m. UTC | #1
On 23/01/2024 13:07, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Doubling 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")
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Cc: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Has this patch been missed?

I have this same build issue for i386 allmodconfig on v6.8-rc5 and earlier

Tested-by: John Garry <john.g.garry@oracle.com> #build only
  
Martin K. Petersen Feb. 23, 2024, 1:16 a.m. UTC | #2
John,

> Has this patch been missed?
>
> I have this same build issue for i386 allmodconfig on v6.8-rc5 and earlier
>
> Tested-by: John Garry <john.g.garry@oracle.com> #build only

Broadcom requested changes and I haven't seen a v3.
  
John Garry Feb. 23, 2024, 8:53 a.m. UTC | #3
On 23/02/2024 01:16, Martin K. Petersen wrote:
> 
> John,
> 
>> Has this patch been missed?
>>
>> I have this same build issue for i386 allmodconfig on v6.8-rc5 and earlier
>>
>> Tested-by: John Garry <john.g.garry@oracle.com> #build only
> 
> Broadcom requested changes and I haven't seen a v3.

 From checking the history, changes were requested for the v1, and Arnd 
addressed them in this v2. Since then, there has been no activity.

Thanks,
John
  
Martin K. Petersen Feb. 27, 2024, 1:35 a.m. UTC | #4
John,

>> Broadcom requested changes and I haven't seen a v3.
>
> From checking the history, changes were requested for the v1, and Arnd
> addressed them in this v2. Since then, there has been no activity.

Interesting. For some reason I kept getting the previous version when I
queried this patch.

Applied to 6.8/scsi-fixes, thanks!
  
Martin K. Petersen Feb. 27, 2024, 2:47 a.m. UTC | #5
On Tue, 23 Jan 2024 14:07:36 +0100, Arnd Bergmann wrote:

> Doubling 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.
> 
> [...]

Applied to 6.8/scsi-fixes, thanks!

[1/1] scsi: mpi3mr: reduce stack usage in mpi3mr_refresh_sas_ports()
      https://git.kernel.org/mkp/scsi/c/5cc2da0b60e5
  

Patch

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index c0c8ab586957..d32ad46318cb 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,10 @@  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(64, 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 +1818,7 @@  mpi3mr_refresh_sas_ports(struct mpi3mr_ioc *mrioc)
 		}
 	}
 out:
+	kfree(h_port);
 	kfree(sas_io_unit_pg0);
 }