Re: [PATCH net-next v3] net: phy: make the hwtstamp provider lifetime symmetric with the PHY

[Chenguang Zhao <chenguang.zhao@linux.dev>]

在 2026/9/30 08:03, Jakub Kicinski 写道:
> On Wed, 23 Sep 2026 15:11:32 +0800 Chenguang Zhao wrote:
>> phy_detach() removes dev->hwprov when it points at the PHY being
>> detached, but phy_attach() does not install anything: the setup and
>> teardown are not symmetric. The teardown path is also reached from
>> the probe error path with no RTNL and an unregistered netdev, making
>> rtnl_dereference() of dev->hwprov trigger a lockdep splat.
>>
>> Track the provider with the PHY instead: dev_attach_hwtstamp_phylib()
>> installs a default provider in phy_attach_direct(), and
>> dev_clear_hwtstamp_phylib() removes it in phy_detach(). Both sit
>> next to dev_set_hwtstamp_phylib() so the locking is obviously the
>> same, relaxed for unregistered netdevs via
>> netdev_ops_lock_dereference_or_invisible(). No driver callback is
>> invoked; installation is skipped when the PHY is not the default
>> hwtstamp provider or when ethtool already set one.
> This is pure slop, you added this incredibly ugly
> netdev_ops_lock_dereference_or_invisible()
> instead of just taking the RCU lock.
>
> Please don't send send any patches to networking for the next 2 months
> and use this time to learn kernel programming basics.

Hi Jakub

You are right. dev->hwprov is an RCU pointer. The probe error path does not hold RTNL, 

and I silenced the lockdep splat by teaching the ops-lock helper that an unregistered netdev 

needs no lock. That was the wrong fix. The access should use rcu_read_lock() with rcu_dereference().


I was trapped in a misleading mindset: I overly focused on making the phy_attach() and phy_detach() 

paths fully symmetric and tried to avoid RTNL lockdep warnings in the error path, which led me to 

invent an inappropriate workaround instead of using the proper RCU pattern.

I will not send networking patches for the next two months. Thank you for pointing this out.


Thanks

Chenguang