Re: [PATCH net-next v4 3/3] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook

[Carlo Szelinsky]

> This patch causes a deadlock with the etop driver (lantiq arx100 soc
> family).

Thanks Aleksander, and thanks for testing it on real hardware. It's a real
deadlock and you're right about the cause.

The cause is that this patch makes phy_device_register() take rtnl_lock()
around the psec attach. etop registers its MDIO bus from ndo_init
(ltq_etop_mdio_init -> mdiobus_register -> mdiobus_scan ->
phy_device_register), and ndo_init already runs under rtnl inside
register_netdevice(). So the second rtnl_lock() deadlocks. It only shows up
on drivers that register the bus from ndo_init - most do it from probe,
outside rtnl - which is why it didn't turn up here earlier.

I don't think we should revert. As Jonas said, this should be fixed in the
patch. My idea for the next version is to stop using rtnl for the psec
attach/detach and use a small dedicated mutex instead. A private lock can't
recurse against the caller's rtnl, so the etop case would go away.

Kory, others - does that sound like the right direction to you?

One thing I want to flag: rtnl there wasn't only guarding the attach, it was
also keeping the ethtool code that reads phydev->psec from racing the detach
that frees it. So the mutex would need to sit in two places - the phy side and
the ethtool paths in net/ethtool/pse-pd.c - otherwise the deadlock is gone but
the use-after-free comes back. Just so we pick the right approach.

Thanks,
Carlo