From: Fijalkowski, Maciej <[email protected]> Sent: Friday, July 10, 2026 11:58 AM
>On Thu, Jul 09, 2026 at 09:37:27AM +0000, Jagielski, Jedrzej wrote: >> From: Simon Horman <[email protected]> >> Sent: Wednesday, July 8, 2026 7:02 PM >> >> >This is an AI-generated review of your patch. The human sending this >> >email has considered the AI review valid, or at least plausible. >> >Full review at: https://netdev-ai.bots.linux.dev/sashiko/ >> >--- >> >ixgbe: E610: add MAC address runtime refresh >> > >> >This change polls RAR0 each service-task cycle on E610 and updates >> >netdev->dev_addr, hw->mac.addr and mac_table[0].addr when firmware has >> >altered the MAC. A refresh is also invoked from ixgbe_reset() so the >> >address survives RAR0 clearing during init_hw(). > >[...] > >> >[Medium] >> >Would it make sense to hoist the E610 gating out of >> >ixgbe_mac_addr_refresh() and skip the rtnl_lock()/rtnl_unlock() >> >entirely for non-E610 devices? >> > >> >The type check lives inside the helper: >> > >> > static void ixgbe_mac_addr_refresh(struct ixgbe_adapter *adapter) >> > { >> > ... >> > if (hw->mac.type != ixgbe_mac_e610) >> > return; >> > ... >> > } >> > >> >so 82598, 82599, X540 and X550 adapters (which comprise most of the >> >ixgbe device family) acquire the global rtnl_mutex every service-task >> >cycle only to return immediately. Something like: >> > >> > if (adapter->hw.mac.type == ixgbe_mac_e610) { >> > rtnl_lock(); >> > ixgbe_mac_addr_refresh(adapter); >> > rtnl_unlock(); >> > } >> >> Cannot introduce rtnl locking into ixgbe_mac_addr_refresh() as this >> can be already called within rtnl-locked path > >i40e does a lot of dancing for cases like this. rtnl lock is handled via >boolean provided as function arg: > >static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired) >... > /* Release the RTNL lock before we start resetting VFs */ > if (!lock_acquired) > rtnl_unlock(); > >... > >end_unlock: > if (!lock_acquired) > rtnl_unlock(); yeah, not the prettiest solution but, agree, should work :) will redesign it, thanks! >> >> > >> >would avoid the rtnl_mutex contention on non-E610 hardware. >> > >> >> ixgbe_reset_subtask(adapter); >> >> ixgbe_phy_interrupt_subtask(adapter); >> >> ixgbe_sfp_detection_subtask(adapter)
