Re: [PATCH iwl-next 4/8] ixgbe: increase SECRX_RDY polling frequency in ixgbe_disable_rx_buff_generic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded or excessively long hardware polling timeout due to using `usleep_range` with a fixed iteration count.
--

commit 8a91d1965e0f71698f5b5eb20f1c8e612eb2947a
Author: Maciej Rabeda <maciej.rabeda@intel.com>

ixgbe: increase SECRX_RDY polling frequency in ixgbe_disable_rx_buff_generic

This commit replaces a 40-iteration busy-wait loop with a 4000-iteration
sleeping loop in ixgbe_disable_rx_buff_generic() to reduce stalls when
the security block is quickly ready.

> The worst-case is now ~80 ms (4000 * 20
> us) plus scheduler latency, which is acceptable since SECRX_RDY
> failing to assert is a non-fatal informational condition (the function
> just logs a debug message).

Is this worst-case calculation accurate?

Because usleep_range() yields the CPU, scheduler latency and timer resolution
limits apply per iteration, rather than once for the entire loop. On a busy
system or one without high-resolution timers, usleep_range(10, 20) might
sleep for 1 to 10 ms per iteration.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> @@ -2683,7 +2683,7 @@ int prot_autoc_write_generic(struct ixgbe_hw *hw, u32 reg_val, bool locked)
>  **/
>  int ixgbe_disable_rx_buff_generic(struct ixgbe_hw *hw)
>  {
> -#define IXGBE_MAX_SECRX_POLL 40
> +#define IXGBE_MAX_SECRX_POLL 4000
>  	int i;
>  	int secrxreg;
>  
> @@ -2695,8 +2695,7 @@ int ixgbe_disable_rx_buff_generic(struct ixgbe_hw *hw)
>  		if (secrxreg & IXGBE_SECRXSTAT_SECRX_RDY)
>  			break;
>  		else
> -			/* Use interrupt-safe sleep just in case */
> -			udelay(1000);
> +			usleep_range(10, 20);
>  	}

Could this loop stall for 4 to 40 seconds if the hardware is unresponsive
and the SECRX_RDY bit fails to assert?

With 4000 iterations, a 1 to 10 ms delay per iteration would result in a
multi-second stall. Since this function is used during device initialization
and reset paths, this could freeze network interface configuration and
potentially trigger khungtaskd panics if the RTNL lock is held.

Would it be safer to use an absolute time check, such as readx_poll_timeout()
or ktime_get(), to enforce a maximum timeout instead of multiplying the loop
count by the requested sleep duration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508031226.3601800-1-aleksandr.loktionov@intel.com?part=4