Re:Re: [PATCH v2 6/7] net: ravb: fix resource teardown ordering

[jiale yao]

Hi Sergey,

Thanks for your review.
You're right — I'll split this into two patches:
One patch for the IRQ/netdev ordering issue.
One patch for the runtime PM resume failure handling.
I'll use --no-git-fallback next time, sorry for this.

At 2026-09-30 03:07:52, "Sergey Shtylyov" <s.shtylyov@xxxxxx> wrote:
>Hello!
>
>   I guess you used scripts/get_maintainer.pl -- if so, I suggest that
>you add --no-git-fallback next time. Your current To: list is painfully
>long and contains some long defunct addresses (like mine)...
>
>On 9/27/26 5:47 PM, Jiale Yao wrote:
>
>> ravb_remove() frees the netdev before devres releases the managed IRQs.
>> The handlers use the netdev as their data pointer, so an interrupt during
>> that window can access freed memory. Probe error paths have the same
>> ordering problem.
>> 
>> The remove callback also returns when runtime resume fails. That leaves
>> the netdev registered while the driver core still releases its managed
>> resources. A running interface already holds a runtime PM reference, so
>> the extra get cannot invoke a failing resume. A resume failure therefore
>> occurs while the interface is down and ndo_stop() will not be called.
>
>   Seems like a separate problem?
>
>> Place the IRQ resources in a dedicated devres group and release it before
>> freeing the netdev. Continue unregistering and freeing software resources
>> when runtime resume fails, but skip the unmatched runtime PM put.
>> 
>> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
>> Fixes: 48f894ab07c4 ("net: ravb: Add runtime PM support")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
>> ---
>>  drivers/net/ethernet/renesas/ravb_main.c | 23 +++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index ea1c7e536791..a25f5ac7062f 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>[...]> @@ -3124,6 +3131,8 @@ static int ravb_probe(struct platform_device *pdev)
>>         pm_runtime_disable(&pdev->dev);
>>         pm_runtime_dont_use_autosuspend(&pdev->dev);
>>         clk_unprepare(priv->refclk);
>> +out_release_irqs:
>
>   Somewhat unobvious label name, given the following call...
>
>> +       devres_release_group(&pdev->dev, priv);
>>  out_reset_assert:
>>         reset_control_assert(rstc);
>>  out_free_netdev:
>> @@ -3141,9 +3150,10 @@ static void ravb_remove(struct platform_device *pdev)
>> 
>>         error = pm_runtime_resume_and_get(dev);
>>         if (error < 0)
>> -               return;
>> +               dev_warn(dev, "failed to resume device: %d\n", error);
>> 
>>         unregister_netdev(ndev);
>> +       devres_release_group(dev, priv);
>>         if (info->nc_queues)
>>                 netif_napi_del(&priv->napi[RAVB_NC]);
>>         netif_napi_del(&priv->napi[RAVB_BE]);
>> @@ -3153,7 +3163,8 @@ static void ravb_remove(struct platform_device *pdev)
>>         dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
>>                           priv->desc_bat_dma);
>> 
>> -       pm_runtime_put_sync_suspend(&pdev->dev);
>> +       if (error >= 0)
>> +               pm_runtime_put_sync_suspend(&pdev->dev);
>
>   Hm, definitely seems like a material for a separate patch...
>
>[...]
>
>MBR, Sergey
Jiale