Re: [PATCH net-next v21 3/5] net: Add the possibility to support a selected hwtstamp in netdevice
From: Claudiu Beznea
Date: Fri Jan 17 2025 - 07:19:31 EST
On 17.01.2025 14:06, Kory Maincent wrote:
> On Fri, 17 Jan 2025 13:57:41 +0200
> Claudiu Beznea <claudiu.beznea@xxxxxxxxx> wrote:
>
>> Hi, Kory,
>>
>> On 12.12.2024 19:06, Kory Maincent wrote:
>>> Introduce the description of a hwtstamp provider, mainly defined with a
>>> the hwtstamp source and the phydev pointer.
>>>
>>> Add a hwtstamp provider description within the netdev structure to
>>> allow saving the hwtstamp we want to use. This prepares for future
>>> support of an ethtool netlink command to select the desired hwtstamp
>>> provider. By default, the old API that does not support hwtstamp
>>> selectability is used, meaning the hwtstamp provider pointer is unset.
>>>
>>> Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>
>>
>> I'm getting this error when doing suspend/resume on the Renesas RZ/G3S
>> Smarc Module + RZ SMARC Carrier II board:
>>
>> [ 39.032969] =============================
>> [ 39.032983] WARNING: suspicious RCU usage
>> [ 39.033000] 6.13.0-rc7-next-20250116-arm64-renesas-00002-g35245dfdc62c
>> #7 Not tainted
>> [ 39.033019] -----------------------------
>> [ 39.033033] drivers/net/phy/phy_device.c:2004 suspicious
>> rcu_dereference_protected() usage!
>
> Thanks for the report.
> Oh so it seems there are cases where phy_detach is not called under RTNL lock!
>
> This should solve the issue:
> - hwprov = rtnl_dereference(dev->hwprov);
> + rcu_read_lock()
> + hwprov = rcu_dereference(dev->hwprov);
> /* Disable timestamp if it is the one selected */
> if (hwprov && hwprov->phydev == phydev) {
> rcu_assign_pointer(dev->hwprov, NULL);
> kfree_rcu(hwprov, rcu_head);
> }
> + rcu_read_unlock();
Just tested. The issue is gone. You can add my:
Tested-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
Thank you,
Claudiu
>
> I will send a patch soon.
>
> Regards,