Messages in this thread | | | Date | Mon, 22 Aug 2022 19:09:25 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net] net: phy: Warn if phy is attached when removing |
| |
> In the last thread I posted this snippet: > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index a74b320f5b27..05894e1c3e59 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -27,6 +27,7 @@ > #include <linux/phy.h> > #include <linux/phy_led_triggers.h> > #include <linux/property.h> > +#include <linux/rtnetlink.h> > #include <linux/sfp.h> > #include <linux/skbuff.h> > #include <linux/slab.h> > @@ -3111,6 +3112,13 @@ static int phy_remove(struct device *dev) > { > struct phy_device *phydev = to_phy_device(dev); > > + // I'm pretty sure this races with multiple unbinds... > + rtnl_lock(); > + device_unlock(dev); > + dev_close(phydev->attached_dev); > + device_lock(dev); > + rtnl_unlock(); > + WARN_ON(phydev->attached_dev); > + > cancel_delayed_work_sync(&phydev->state_queue); > > mutex_lock(&phydev->lock); > --- > > Would this be acceptable? Can the locking be fixed?
Code like this should not be hidden in the PHY layer. If we decide to go down this path it probably should be in net/core/dev.c.
I suggest you talk to the maintainers of that file, probably Eric Dumazet, give him a clear explanation of the problem, and see what he suggests.
Andrew
| |