lkml.org 
[lkml]   [2022]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-08-22 19:11    [W:0.548 / U:0.820 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site