Re: [PATCH net-next v10 4/5] net: wangxun: implement soft quiesce for PCIe error recovery
From: Simon Horman <horms@kernel.org>
Date: 2026-07-14 09:11:26
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://sashiko.dev ---
--- Patch [4]: [PATCH net-next v10 4/5] net: wangxun: implement soft quiesce for PCIe error recovery ---commit 1b8a0e5e722ffb89c8fe24492e80dca44f3410f2 Author: Jiawen Wu [off-list ref] net: wangxun: implement soft quiesce for PCIe error recovery This commit introduces a soft quiesce path for PCIe error recovery. It adds WX_STATE_DOWN and WX_STATE_RESETTING state checks to subtasks to block execution, explicitly avoiding a cancellation of the service task to speed up recovery.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c index c8a4822d38194..a65312c8078fa 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3383,5 +3383,23 @@ void wx_service_timer(struct timer_list *t) } EXPORT_SYMBOL(wx_service_timer); +void wx_soft_quiesce(struct wx *wx) +{ + if (!netif_running(wx->netdev) || + test_and_set_bit(WX_STATE_DOWN, wx->state)) + return; + + pci_clear_master(wx->pdev); + netif_tx_stop_all_queues(wx->netdev); + netif_carrier_off(wx->netdev); + netif_tx_disable(wx->netdev); + wx_napi_disable_all(wx); + wx_ptp_quiesce(wx); + + clear_bit(WX_FLAG_NEED_PF_RESET, wx->flags); + timer_delete_sync(&wx->service_timer);
[Severity: High] The commit message notes the service task is deliberately not cancelled here to avoid delaying PCIe error recovery. However, does omitting cancel_work_sync(&wx->service_task) open a race window with the subtasks that rely on the WX_STATE_DOWN flag?
+} +EXPORT_SYMBOL(wx_soft_quiesce);
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c index a8773712cff82..a7bde03a98feb 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -94,6 +94,10 @@ static void txgbe_module_detection_subtask(struct wx *wx) { int err; + if (test_bit(WX_STATE_DOWN, wx->state) || + test_bit(WX_STATE_RESETTING, wx->state)) + return; +
[Severity: High]
Does this state check adequately protect against concurrent MMIO access
during a PCIe reset?
If the task passes this check, it subsequently sleeps for 200ms:
txgbe_main.c:txgbe_module_detection_subtask() {
...
if (!test_and_clear_bit(WX_FLAG_NEED_MODULE_RESET, wx->flags))
return;
/* wait for SFF module ready */
msleep(200);
err = txgbe_identify_module(wx);
...
}
If wx_soft_quiesce() runs on another CPU during this msleep(), it will set
WX_STATE_DOWN and begin a hardware reset.
When the task wakes up from the sleep, will it call txgbe_identify_module()
and perform MMIO reads (such as rd32(wx, WX_GPIO_EXT)) while the device is
in reset?
Could this time-of-check to time-of-use race lead to Master Aborts or
fatal PCIe errors during AER recovery?
if (!test_and_clear_bit(WX_FLAG_NEED_MODULE_RESET, wx->flags)) return;