Re: [PATCH net] net: phy: Warn if phy is attached when removing

[Andrew Lunn <andrew@lunn.ch>]

> 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