[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9f1486ff-2351-4002-8ee6-e2cfad354251@oracle.com>
Date: Fri, 20 Jun 2025 19:04:41 +0530
From: ALOK TIWARI <alok.a.tiwari@...cle.com>
To: Arnd Bergmann <arnd@...nel.org>, Nilesh Javali <njavali@...vell.com>,
GR-QLogic-Storage-Upstream@...vell.com,
"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>
Cc: Arnd Bergmann <arnd@...db.de>, Quinn Tran <qutran@...vell.com>,
Himanshu Madhani <himanshu.madhani@...cle.com>,
"Dr. David Alan Gilbert" <linux@...blig.org>,
linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scsi: qla2xxx: avoid stack frame size warning in qla_dfs
On 6/20/2025 5:09 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.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@...db.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
Powered by blists - more mailing lists