[PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

bfa_fcs_fdmi_get_portattr() gets inlined into multiple places and has two
fairly large variables on the stack, to the point of causing a warning
in some randconfig builds:

drivers/scsi/bfa/bfa_fcs_lport.c:2198:1: error: stack frame size (1560) exceeds limit (1280) in 'bfa_fcs_lport_fdmi_build_portattr_block' [-Werror,-Wframe-larger-than]
 2198 | bfa_fcs_lport_fdmi_build_portattr_block(struct bfa_fcs_lport_fdmi_s *fdmi,
      | ^
drivers/scsi/bfa/bfa_fcs_lport.c:1856:1: error: stack frame size (1600) exceeds limit (1280) in 'bfa_fcs_lport_fdmi_build_rhba_pyld' [-Werror,-Wframe-larger-than]
 1856 | bfa_fcs_lport_fdmi_build_rhba_pyld(struct bfa_fcs_lport_fdmi_s *fdmi, u8 *pyld)
      | ^

Mark the inner function as noinline_for_stack to keep it separate from
the other variables and prevent multiple copies of the same variable
to get inlined here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_fcs_lport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
index 2df399c537c1..8c9d423129c0 100644
--- a/drivers/scsi/bfa/bfa_fcs_lport.c
+++ b/drivers/scsi/bfa/bfa_fcs_lport.c
@@ -2627,7 +2627,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
 
 }
 
-static void
+static noinline_for_stack void
 bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
 			  struct bfa_fcs_fdmi_port_attr_s *port_attr)
 {
-- 
2.39.5

Re: [PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled error path leaves the state machine stalled, leading to a leaked fcxp structure and a list_del() crash upon port offline.
--

commit 61029f85cbc6b31d41ea7d38cd2303b673cd3f35
Author: Arnd Bergmann <arnd@arndb.de>

scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

bfa_fcs_fdmi_get_portattr() gets inlined into multiple places and has two
fairly large variables on the stack, to the point of causing a warning
in some randconfig builds. Mark the inner function as noinline_for_stack
to keep it separate from the other variables.

> diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
> index 2df399c537c13..8c9d423129c07 100644
> --- a/drivers/scsi/bfa/bfa_fcs_lport.c
> +++ b/drivers/scsi/bfa/bfa_fcs_lport.c
> @@ -2627,7 +2627,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
>  
>  }
>  
> -static void
> +static noinline_for_stack void
>  bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
>  			  struct bfa_fcs_fdmi_port_attr_s *port_attr)
>  {

[Severity: High]
This is a pre-existing issue, but I have a question about the error path in
bfa_fcs_lport_fdmi_send_rhba() when it calls bfa_fcs_lport_fdmi_build_rhba_pyld():

drivers/scsi/bfa/bfa_fcs_lport.c:bfa_fcs_lport_fdmi_send_rhba() {
    ...
	attr_len =
		bfa_fcs_lport_fdmi_build_rhba_pyld(fdmi,
					  (u8 *) ((struct ct_hdr_s *) pyld
						       + 1));
	if (attr_len < 0)
		return;
    ...
}

If this returns early due to a negative error code, does it leak the
successfully allocated fcxp structure?

It appears that returning early here fails to free the fcxp structure and does
not advance the state machine, leaving it stalled in the
bfa_fcs_lport_fdmi_sm_sending_rhba state.

If a port offline event subsequently occurs, the state machine blindly cancels
the allocation:

drivers/scsi/bfa/bfa_fcs_lport.c:bfa_fcs_lport_fdmi_sm_sending_rhba() {
    ...
	case FDMISM_EVENT_PORT_OFFLINE:
		bfa_sm_set_state(fdmi, bfa_fcs_lport_fdmi_sm_offline);
		bfa_fcxp_walloc_cancel(BFA_FCS_GET_HAL_FROM_PORT(port),
					   &fdmi->fcxp_wqe);
		break;
    ...
}

Since fcxp was originally successfully allocated, the wait queue entry
(fdmi->fcxp_wqe.qe) was never linked onto the wait list. Can calling
bfa_fcxp_walloc_cancel() on this unqueued and uninitialized element trigger a
list_del() crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611125601.3385418-1-arnd@kernel.org?part=1

Re: [PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[Arnd Bergmann]

On Thu, Jun 11, 2026, at 15:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential 
> issue(s) to consider:
>
> Pre-existing issues:
> - [High] Unhandled error path leaves the state machine stalled, leading 
> to a leaked fcxp structure and a list_del() crash upon port offline.
> --

Correct, this came in from a7a11b6cfec2 ("scsi: bfa: Move a large
struct from the stack onto the heap"). My first attempt at fixing
the new warning was to take the same approach that Lee did at
the time. A dynamic allocation would have been better for the
stack usage, but I ended up running into the exact same problem
as the earlier patch and decided against this. I had not realized
that this was already an issue.

As I had nothing to do with that fix, I'm not going to attempt
to fix it, but I would suggest that the maintainers take a
look at fixing the error handling. If the same method is
used in bfa_fcs_fdmi_get_hbaattr() and
bfa_fcs_lport_fdmi_build_portattr_block() to avoid the
large stack variables, my patch is no longer needed.

    Arnd

Re: [PATCH] scsi: bfa: reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block

[Martin K. Petersen]

Arnd,

> bfa_fcs_fdmi_get_portattr() gets inlined into multiple places and has
> two fairly large variables on the stack, to the point of causing a
> warning in some randconfig builds:

Applied to 7.2/scsi-staging, thanks!

-- 
Martin K. Petersen