RE: [PATCH net] net: txgbe: fix RTNL assertion warning when remove module

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Tue, Apr 7, 2026 3:41 PM, Russell King (Oracle) wrote:
> On Tue, Apr 07, 2026 at 02:27:34PM +0800, Jiawen Wu wrote:
> > On Wed, Apr 1, 2026 2:22 PM, Russell King (Oracle) wrote:
> > > On Wed, Apr 01, 2026 at 10:16:34AM +0800, Jiawen Wu wrote:
> > > > On Tue, Mar 31, 2026 9:08 PM, Russell King (Oracle) wrote:
> > > > > On Tue, Mar 31, 2026 at 03:11:07PM +0800, Jiawen Wu wrote:
> > > > > > For the copper NIC with external PHY, the driver called
> > > > > > phylink_connect_phy() during probe and phylink_disconnect_phy() during
> > > > > > remove. It caused an RTNL assertion warning in phylink_disconnect_phy()
> > > > > > upon module remove.
> > > > > >
> > > > > > To fix this, move the phylink connect/disconnect PHY to ndo_open/close.
> > > > >
> > > > > Wouldn't it be simpler to just wrap the phylink_disconnect_phy() in the
> > > > > remove function with rtnl_lock()..rtnl_unlock() ?
> > > >
> > > > This is also a solution. But I think it would be nice to unify with other drivers
> > > > that call the functions in ndo_open/close.
> > >
> > > Both approaches are equally valid. Some network drivers attach the PHY
> > > at probe time (and thus can return -EPROBE_DEFER if the PHY is specified
> > > but not present). Others attach in .ndo_open which can only fail in this
> > > circumstance with no retry without userspace manually implementing that.
> > >
> > > There are other advantages and disadvantages to each approach.
> >
> > Hi,
> >
> > So is it still recommended that add rtnl_lock()...rtnl_unlock() instead of moving it?
> 
> The reaosn phylink_disconnect_phy() requires the RTNL lock is because it
> _can_ be called while the netdev is published, and the RTNL lock
> protects the networking core from the PHY being removed from the netdev,
> preventing ethtool ops into the PHY driver from running concurrently
> with the PHY's disconnection and potential later destruction.
> 
> Offering two APIs, one which requires the lock to provide that
> protection and one which doesn't would over-complicate the phylink code
> and make reviews way more difficult, as we'd now have to spot the
> wrong function being used in the wrong code path.
> 
> It's simpler for drivers that want to connect and disconnect the PHY
> at probe/remove time for them to just take the RTNL lock briefly over
> the call to phylink_disconnect_phy().
> 
> There is no "recommendation" for connecting and disconnecting the
> PHY at probe/remove time vs ndo_open/ndo_release. That's entirely up
> to the driver author. As I've already said, there are advantages and
> disadvantages of either way and that's a matter for the driver author
> to consider and select the most appropriate choice for their driver.

OK. I'll change it at v2 patch.