[PATCH] scsi: megaraid: reduce stack usage in megaraid_cmm_register()

Arnd Bergmann posted 1 patch 5 days, 4 hours ago
drivers/scsi/megaraid/megaraid_mbox.c | 26 ++++++++++++++---------
drivers/scsi/megaraid/megaraid_mm.c   | 30 +++++++--------------------
2 files changed, 24 insertions(+), 32 deletions(-)
[PATCH] scsi: megaraid: reduce stack usage in megaraid_cmm_register()
Posted by Arnd Bergmann 5 days, 4 hours ago
From: Arnd Bergmann <arnd@arndb.de>

The megaraid_cmm_register() function has a local copy of mraid_mmadp_t on
the stack that gets copied into the actual structure used at runtime. When
-fsanitize=thread is enabled, this causes the per-function stack frame
to grow beyond the warning limit:

megaraid_mbox.c: In function 'megaraid_cmm_register':
megaraid_mbox.c:3472:1: error: the frame size of 1312 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

Refactor this by moving the allocation into the caller to
save the extra on-stack copy of the structure.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/megaraid/megaraid_mbox.c | 26 ++++++++++++++---------
 drivers/scsi/megaraid/megaraid_mm.c   | 30 +++++++--------------------
 2 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
index 06cf94ee4e36..ce89032a5a74 100644
--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -3396,7 +3396,7 @@ static int
 megaraid_cmm_register(adapter_t *adapter)
 {
 	mraid_device_t	*raid_dev = ADAP2RAIDDEV(adapter);
-	mraid_mmadp_t	adp;
+	mraid_mmadp_t	*adp;
 	scb_t		*scb;
 	mbox_ccb_t	*ccb;
 	int		rval;
@@ -3404,11 +3404,16 @@ megaraid_cmm_register(adapter_t *adapter)
 
 	// Allocate memory for the base list of scb for management module.
 	adapter->uscb_list = kzalloc_objs(scb_t, MBOX_MAX_USER_CMDS);
+	adp = kzalloc_obj(*adp);
 
-	if (adapter->uscb_list == NULL) {
+	if (!adapter->uscb_list || !adp) {
 		con_log(CL_ANN, (KERN_WARNING
 			"megaraid: out of memory, %s %d\n", __func__,
 			__LINE__));
+
+		kfree(adapter->uscb_list);
+		kfree(adp);
+
 		return -1;
 	}
 
@@ -3452,20 +3457,21 @@ megaraid_cmm_register(adapter_t *adapter)
 		list_add_tail(&scb->list, &adapter->uscb_pool);
 	}
 
-	adp.unique_id		= adapter->unique_id;
-	adp.drvr_type		= DRVRTYPE_MBOX;
-	adp.drvr_data		= (unsigned long)adapter;
-	adp.pdev		= adapter->pdev;
-	adp.issue_uioc		= megaraid_mbox_mm_handler;
-	adp.timeout		= MBOX_RESET_WAIT + MBOX_RESET_EXT_WAIT;
-	adp.max_kioc		= MBOX_MAX_USER_CMDS;
+	adp->unique_id		= adapter->unique_id;
+	adp->drvr_type		= DRVRTYPE_MBOX;
+	adp->drvr_data		= (unsigned long)adapter;
+	adp->pdev		= adapter->pdev;
+	adp->issue_uioc		= megaraid_mbox_mm_handler;
+	adp->timeout		= MBOX_RESET_WAIT + MBOX_RESET_EXT_WAIT;
+	adp->max_kioc		= MBOX_MAX_USER_CMDS;
 
-	if ((rval = mraid_mm_register_adp(&adp)) != 0) {
+	if ((rval = mraid_mm_register_adp(adp)) != 0) {
 
 		con_log(CL_ANN, (KERN_WARNING
 			"megaraid mbox: did not register with CMM\n"));
 
 		kfree(adapter->uscb_list);
+		kfree(adp);
 	}
 
 	return rval;
diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c
index 538da0e98131..60db48dc8f3a 100644
--- a/drivers/scsi/megaraid/megaraid_mm.c
+++ b/drivers/scsi/megaraid/megaraid_mm.c
@@ -898,42 +898,28 @@ hinfo_to_cinfo(mraid_hba_info_t *hinfo, mcontroller_t *cinfo)
 
 /**
  * mraid_mm_register_adp - Registration routine for low level drivers
- * @lld_adp	: Adapter object
+ * @adapter	: Adapter object
  */
 int
-mraid_mm_register_adp(mraid_mmadp_t *lld_adp)
+mraid_mm_register_adp(mraid_mmadp_t *adapter)
 {
-	mraid_mmadp_t	*adapter;
 	mbox64_t	*mbox_list;
 	uioc_t		*kioc;
 	uint32_t	rval;
 	int		i;
 
 
-	if (lld_adp->drvr_type != DRVRTYPE_MBOX)
+	if (adapter->drvr_type != DRVRTYPE_MBOX)
 		return (-EINVAL);
 
-	adapter = kzalloc_obj(mraid_mmadp_t);
-
-	if (!adapter)
-		return -ENOMEM;
-
-
-	adapter->unique_id	= lld_adp->unique_id;
-	adapter->drvr_type	= lld_adp->drvr_type;
-	adapter->drvr_data	= lld_adp->drvr_data;
-	adapter->pdev		= lld_adp->pdev;
-	adapter->issue_uioc	= lld_adp->issue_uioc;
-	adapter->timeout	= lld_adp->timeout;
-	adapter->max_kioc	= lld_adp->max_kioc;
 	adapter->quiescent	= 1;
 
 	/*
 	 * Allocate single blocks of memory for all required kiocs,
 	 * mailboxes and passthru structures.
 	 */
-	adapter->kioc_list	= kmalloc_objs(uioc_t, lld_adp->max_kioc);
-	adapter->mbox_list	= kmalloc_objs(mbox64_t, lld_adp->max_kioc);
+	adapter->kioc_list	= kmalloc_objs(uioc_t, adapter->max_kioc);
+	adapter->mbox_list	= kmalloc_objs(mbox64_t, adapter->max_kioc);
 	adapter->pthru_dma_pool = dma_pool_create("megaraid mm pthru pool",
 						&adapter->pdev->dev,
 						sizeof(mraid_passthru_t),
@@ -956,11 +942,11 @@ mraid_mm_register_adp(mraid_mmadp_t *lld_adp)
 	 */
 	INIT_LIST_HEAD(&adapter->kioc_pool);
 	spin_lock_init(&adapter->kioc_pool_lock);
-	sema_init(&adapter->kioc_semaphore, lld_adp->max_kioc);
+	sema_init(&adapter->kioc_semaphore, adapter->max_kioc);
 
 	mbox_list	= (mbox64_t *)adapter->mbox_list;
 
-	for (i = 0; i < lld_adp->max_kioc; i++) {
+	for (i = 0; i < adapter->max_kioc; i++) {
 
 		kioc		= adapter->kioc_list + i;
 		kioc->cmdbuf	= (uint64_t)(unsigned long)(mbox_list + i);
@@ -997,7 +983,7 @@ mraid_mm_register_adp(mraid_mmadp_t *lld_adp)
 
 pthru_dma_pool_error:
 
-	for (i = 0; i < lld_adp->max_kioc; i++) {
+	for (i = 0; i < adapter->max_kioc; i++) {
 		kioc = adapter->kioc_list + i;
 		if (kioc->pthru32) {
 			dma_pool_free(adapter->pthru_dma_pool, kioc->pthru32,
-- 
2.39.5
Re: [PATCH] scsi: megaraid: reduce stack usage in megaraid_cmm_register()
Posted by Martin K. Petersen 1 day, 22 hours ago
Arnd,

> The megaraid_cmm_register() function has a local copy of mraid_mmadp_t
> on the stack that gets copied into the actual structure used at
> runtime. When -fsanitize=thread is enabled, this causes the
> per-function stack frame to grow beyond the warning limit:

Applied to 7.2/scsi-staging, thanks!

-- 
Martin K. Petersen