__igb_open() links queues to NAPIs with netif_queue_set_napi(), which requires RTNL, but RTNL isn't held when resuming from runtime suspend: this triggers "RTNL: assertion failed". Don't link them in __igb_open() when resuming: do it right away on system resume (RTNL held) and from a work item taking RTNL on runtime resume. The links can't be kept across a suspend, because __igb_shutdown() frees the q_vectors. These are only freed under RTNL with __IGB_DOWN set, so the work item checks __IGB_DOWN under RTNL. Fixes: b75a1dea500f ("igb: Link queues to NAPI instances") Cc: stable@vger.kernel.org # 6.16+ Closes: https://bugzilla.ipfire.org/show_bug.cgi?id=13562 Signed-off-by: Antonio Quartulli --- Note: igc may have the same issue: __igc_open() also links queues to NAPIs (igc_set_queue_napi()) on the RTNL-less runtime resume path. We didn't have igc hardware to test a fix with, so no igc patch is sent for now. drivers/net/ethernet/intel/igb/igb.h | 1 + drivers/net/ethernet/intel/igb/igb_main.c | 38 +++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 0fff1df..ea2d739 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -585,6 +585,7 @@ struct igb_adapter { struct work_struct reset_task; struct work_struct watchdog_task; + struct work_struct queue_napi_task; bool fc_autoneg; u8 tx_timeout_factor; struct timer_list blink_timer; diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index bd1185d..4d97399 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -2115,6 +2115,30 @@ void igb_set_queue_napi(struct igb_adapter *adapter, int vector, NETDEV_QUEUE_TYPE_TX, napi); } +static void igb_link_queue_napis(struct igb_adapter *adapter) +{ + int i; + + for (i = 0; i < adapter->num_q_vectors; i++) + igb_set_queue_napi(adapter, i, &adapter->q_vector[i]->napi); +} + +/* Linking queues to NAPIs requires RTNL, which isn't held when resuming + * from runtime suspend: do it from here instead. The q_vectors are only + * freed under RTNL with __IGB_DOWN set, so checking it under RTNL is safe. + */ +static void igb_queue_napi_task(struct work_struct *work) +{ + struct igb_adapter *adapter = container_of(work, struct igb_adapter, + queue_napi_task); + + rtnl_lock(); + if (!test_bit(__IGB_DOWN, &adapter->state) && + netif_running(adapter->netdev)) + igb_link_queue_napis(adapter); + rtnl_unlock(); +} + /** * igb_up - Open the interface and prepare it to handle traffic * @adapter: board private structure @@ -3426,6 +3450,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent) INIT_WORK(&adapter->reset_task, igb_reset_task); INIT_WORK(&adapter->watchdog_task, igb_watchdog_task); + INIT_WORK(&adapter->queue_napi_task, igb_queue_napi_task); /* Initialize link properties that are user-changeable */ adapter->fc_autoneg = true; @@ -3890,6 +3915,7 @@ static void igb_remove(struct pci_dev *pdev) cancel_work_sync(&adapter->reset_task); cancel_work_sync(&adapter->watchdog_task); + cancel_work_sync(&adapter->queue_napi_task); #ifdef CONFIG_IGB_DCA if (adapter->flags & IGB_FLAG_DCA_ENABLED) { @@ -4184,7 +4210,8 @@ static int __igb_open(struct net_device *netdev, bool resuming) for (i = 0; i < adapter->num_q_vectors; i++) { napi = &adapter->q_vector[i]->napi; napi_enable(napi); - igb_set_queue_napi(adapter, i, napi); + if (!resuming) + igb_set_queue_napi(adapter, i, napi); } /* Clear any pending interrupts. */ @@ -9631,8 +9658,15 @@ static int __igb_resume(struct device *dev, bool rpm) if (!rpm) rtnl_lock(); - if (!err && netif_running(netdev)) + if (!err && netif_running(netdev)) { err = __igb_open(netdev, true); + if (!err) { + if (rpm) + schedule_work(&adapter->queue_napi_task); + else + igb_link_queue_napis(adapter); + } + } if (!err) netif_device_attach(netdev); -- 2.56.0