Re: [PATCH] scsi: qla2xxx: avoid stack frame size warning in qla_dfs

[ALOK TIWARI <alok.a.tiwari@oracle.com>]

On 6/20/2025 5:09 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The qla2x00_dfs_tgt_port_database_show() function constructs a fake
> fc_port_t object on the stack, which depending on the configuration
> is large enough to exceed the stack size warning limit:
> 
> drivers/scsi/qla2xxx/qla_dfs.c:176:1: error: stack frame size (1392) exceeds limit (1280) in 'qla2x00_dfs_tgt_port_database_show' [-Werror,-Wframe-larger-than]
> 
> Rework this function to no longer need the structure but instead
> call a custom helper function that just prints the data directly
> from the port_database_24xx structure.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I don't have this hardware and tried restructing the code in
> a way that makes sense to me. This should be tested on an actual
> device before it gets applied.
> ---
>   drivers/scsi/qla2xxx/qla_dfs.c | 20 +++++---------
>   drivers/scsi/qla2xxx/qla_gbl.h |  1 +
>   drivers/scsi/qla2xxx/qla_mbx.c | 49 ++++++++++++++++++++++++++++++++++
>   3 files changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
> index 08273520c777..43970caca7b3 100644
> --- a/drivers/scsi/qla2xxx/qla_dfs.c
> +++ b/drivers/scsi/qla2xxx/qla_dfs.c
> @@ -179,10 +179,9 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
>   	struct qla_hw_data *ha = vha->hw;
>   	struct gid_list_info *gid_list;
>   	dma_addr_t gid_list_dma;
> -	fc_port_t fc_port;
>   	char *id_iter;
>   	int rc, i;
> -	uint16_t entries, loop_id;
> +	uint16_t entries;
>   
>   	seq_printf(s, "%s\n", vha->host_str);
>   	gid_list = dma_alloc_coherent(&ha->pdev->dev,
> @@ -205,18 +204,11 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
>   	seq_puts(s, "Port Name	Port ID		Loop ID\n");
>   
>   	for (i = 0; i < entries; i++) {
> -		struct gid_list_info *gid =
> -			(struct gid_list_info *)id_iter;
> -		loop_id = le16_to_cpu(gid->loop_id);
> -		memset(&fc_port, 0, sizeof(fc_port_t));
> -
> -		fc_port.loop_id = loop_id;
> -
> -		rc = qla24xx_gpdb_wait(vha, &fc_port, 0);
> -		seq_printf(s, "%8phC  %02x%02x%02x  %d\n",
> -			   fc_port.port_name, fc_port.d_id.b.domain,
> -			   fc_port.d_id.b.area, fc_port.d_id.b.al_pa,
> -			   fc_port.loop_id);
> +		struct gid_list_info *gid = (struct gid_list_info *)id_iter;
> +
> +		rc = qla24xx_print_fc_port_id(vha, s, le16_to_cpu(gid->loop_id));
> +		if (rc != QLA_SUCCESS)
> +			break;
>   		id_iter += ha->gid_list_info_size;
>   	}
>   out_free_id_list:
> diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
> index 03e50e8fc08d..145defc420f2 100644
> --- a/drivers/scsi/qla2xxx/qla_gbl.h
> +++ b/drivers/scsi/qla2xxx/qla_gbl.h
> @@ -557,6 +557,7 @@ qla26xx_dport_diagnostics_v2(scsi_qla_host_t *,
>   
>   int qla24xx_send_mb_cmd(struct scsi_qla_host *, mbx_cmd_t *);
>   int qla24xx_gpdb_wait(struct scsi_qla_host *, fc_port_t *, u8);
> +int qla24xx_print_fc_port_id(struct scsi_qla_host *, struct seq_file *, u16);
>   int qla24xx_gidlist_wait(struct scsi_qla_host *, void *, dma_addr_t,
>       uint16_t *);
>   int __qla24xx_parse_gpdb(struct scsi_qla_host *, fc_port_t *,
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index 0cd6f3e14882..a51b9704cf4b 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -6597,6 +6597,55 @@ int qla24xx_send_mb_cmd(struct scsi_qla_host *vha, mbx_cmd_t *mcp)
>   	return rval;
>   }
>   
> +int qla24xx_print_fc_port_id(struct scsi_qla_host *vha, struct seq_file *s, u16 loop_id)
> +{
> +	int rval = QLA_FUNCTION_FAILED;
> +	dma_addr_t pd_dma;
> +	struct port_database_24xx *pd;
> +	struct qla_hw_data *ha = vha->hw;
> +	mbx_cmd_t mc;
> +
> +	if (!vha->hw->flags.fw_started)
> +		goto done;
> +
> +	pd = dma_pool_zalloc(ha->s_dma_pool, GFP_KERNEL, &pd_dma);
> +	if (pd  == NULL) {

remove extra " " after pd

> +		ql_log(ql_log_warn, vha, 0xd047,
> +		    "Failed to allocate port database structure.\n");
> +		goto done_free_sp;

-> goto done
as pd is NULL it wont called dma_pool_free at all.
dma_pool_free, trying to free a pd (NULL) confusing.

> +	}
> +
> +	memset(&mc, 0, sizeof(mc));
> +	mc.mb[0] = MBC_GET_PORT_DATABASE;
> +	mc.mb[1] = loop_id;
> +	mc.mb[2] = MSW(pd_dma);
> +	mc.mb[3] = LSW(pd_dma);
> +	mc.mb[6] = MSW(MSD(pd_dma));
> +	mc.mb[7] = LSW(MSD(pd_dma));
> +	mc.mb[9] = vha->vp_idx;
> +	mc.mb[10] = 0;

seems Redundant, as already called memset ?

> +
> +	rval = qla24xx_send_mb_cmd(vha, &mc);
> +	if (rval != QLA_SUCCESS) {
> +		ql_dbg(ql_dbg_mbx, vha, 0x1193, "%s: fail\n", __func__);
> +		goto done_free_sp;
> +	}
> +
> +	ql_dbg(ql_dbg_mbx, vha, 0x1197, "%s: %8phC done\n",
> +	    __func__, pd->port_name);
> +
> +	seq_printf(s, "%8phC  %02x%02x%02x  %d\n",
> +		   pd->port_name, pd->port_id[0],
> +		   pd->port_id[1], pd->port_id[2],
> +		   loop_id);
> +
> +done_free_sp:
> +	if (pd)
> +		dma_pool_free(ha->s_dma_pool, pd, pd_dma);
> +done:
> +	return rval;
> +}
> +
>   /*
>    * qla24xx_gpdb_wait
>    * NOTE: Do not call this routine from DPC thread

Thanks,
Alok