Re: [PATCH net] net: mana: fix reset work race with device removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch introduces a 10-second sleep while holding the device core lock (`device_lock`), which will block system PM and driver core operations.

Pre-existing issues:
- [High] Driver unbind unexpectedly triggers a full PCI bus rescan due to the `!gc` check.
--

--- Patch [1]: [PATCH net] net: mana: fix reset work race with device removal ---
commit 839c641df5112c8f696d580233f5a31b4e6686dd
Author: Fan Wu <fanwu01@zju.edu.cn>

net: mana: fix reset work race with device removal

The reset service work runs on the system workqueue and obtains the
GDMA context through PCI drvdata. It can race with device removal,
which frees the context, so this patch serializes mana_serv_reset()
with device removal by taking the PCI device lock.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -591,15 +591,16 @@ static void mana_serv_fpga(struct pci_dev *pdev)
>  
>  static void mana_serv_reset(struct pci_dev *pdev)
>  {
> -	struct gdma_context *gc = pci_get_drvdata(pdev);
> +	struct gdma_context *gc;
>  	struct hw_channel_context *hwc;
>  	int ret;
>  
> +	device_lock(&pdev->dev);

[Severity: High]
Does holding device_lock() here cause a regression by stalling system PM
and driver core operations?

mana_serv_reset() calls msleep(MANA_SERVICE_PERIOD * 1000) (which is a 10
second sleep) further down in this function:

mana_serv_reset() {
        ...
        mana_gd_suspend(pdev, PMSG_SUSPEND);
        msleep(MANA_SERVICE_PERIOD * 1000);
        ...
}

Because system suspend loops over all devices and sequentially acquires
device_lock, would a suspend attempt or a user-space unbind operation
unconditionally hang for up to 10 seconds while waiting for this lock?

> +	gc = pci_get_drvdata(pdev);
>  	if (!gc) {
>  		/* Perform PCI rescan on device if GC is not set up */
>  		dev_err(&pdev->dev, "MANA service: GC not setup, rescanning\n");
> -		mana_serv_rescan(pdev);
> -		return;
> +		goto rescan;

[Severity: High]
This is a pre-existing issue, but if mana_gd_remove() is called due to a
standard driver unbind, it clears drvdata. If a pending reset work then
executes, acquires the lock, and observes gc == NULL, it jumps to the rescan
label which forcefully removes the device and triggers a full PCI bus rescan.

Will this effectively defeat the user's attempt to unbind the driver and
disrupt the PCI topology?

>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805143812.220509-1-fanwu01@zju.edu.cn?part=1