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 <[email protected]> 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/[email protected]?part=4
