net: dsa: add missing calls in dsa_switch_destroy
Subsystems (3)
PATCH v2 5/6 net: dsa: add missing calls in dsa_switch_destroy
Add missing netif_carrier_off and phy_disconnect calls to the dsa_switch_destroy function to make sure the netdev and phy ressources are clean before complete removal.
--- net/dsa/dsa.c | 2 ++ 1 file changed, 2 insertions(+)
net/dsa/dsa.c +2 -0
@@ -454,7 +454,9 @@ static void dsa_switch_destroy(struct dsa_switch *ds)if (!ds->ports[port])continue ; +netif_carrier_off (ds->ports[port]);unregister_netdev (ds->ports[port]); +phy_disconnect (p->phy);free_netdev (ds->ports[port]); }
Signature · 1 line
1.9.1
kbuild test robot
build botreproduce:
Hi Neil,
[auto build test ERROR on net/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Neil-Armstrong/Further-fix-for-dsa-unbinding/20151029-212633 config: x86_64-randconfig-x010-201543 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64
Note: the linux-review/Neil-Armstrong/Further-fix-for-dsa-unbinding/20151029-212633 HEAD 3c957c00de125eeffe9f50eda7cf94a98855f615 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
net/dsa/dsa.c: In function 'dsa_switch_destroy':
>> net/dsa/dsa.c:459:18: error: 'p' undeclared (first use in this function)
phy_disconnect(p->phy);
^ net/dsa/dsa.c:459:18: note: each undeclared identifier is reported only once for each function it appears in
vim +/p +459 net/dsa/dsa.c
453
454 if (!ds->ports[port])
455 continue;
456
457 netif_carrier_off(ds->ports[port]);
458 unregister_netdev(ds->ports[port]);
> 459 phy_disconnect(p->phy);
460 free_netdev(ds->ports[port]);
461 }
462
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Andrew Lunn
maintainerQuoted 15 lines · Neil Armstrong
Add missing
netif_carrier_offandphy_disconnectcalls to thedsa_switch_destroyfunction to make sure the netdev and phy ressources are clean before complete removal.Signed-off-by Frode Isaksen <[email protected]>Signed-off-by Neil Armstrong <[email protected]>--- net/dsa/dsa.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/dsa.c +0
-0
net/dsa/dsa.c static void dsa_switch_destroy(struct dsa_switch *ds) · line 461
Quoted 8 lines
net/dsa/dsa.c +2
-0@@ -454,7 +454,9 @@ static void dsa_switch_destroy(struct dsa_switch *ds)if (!ds->ports[port])continue ; +netif_carrier_off (ds->ports[port]);unregister_netdev (ds->ports[port]); +phy_disconnect (p->phy);free_netdev (ds->ports[port]); }
Once you make it actually compile....
I'm not sure this is safe. The loop above this one has just destroyed some phys, and now you are potentially disconnecting a phy you just destroyed, causing it to be accessed?
I would suggest you first fix the ordering in dsa_switch_destroy() and then add the missing netif_carrier_off() and phy_disconnect().
Thanks
Andrew
Quoted 14 lines · Andrew Lunn
net/dsa/dsa.c static void dsa_switch_destroy(struct dsa_switch *ds) · line 461
Quoted 6 lines · Neil Armstrong
net/dsa/dsa.c +2
-0+netif_carrier_off (ds->ports[port]);unregister_netdev (ds->ports[port]); +phy_disconnect (p->phy);free_netdev (ds->ports[port]); }Once you make it actually compile....
I'm not sure this is safe. The loop above this one has just destroyed some phys, and now you are potentially disconnecting a phy you just destroyed, causing it to be accessed?
Quoted 6 lines
I would suggest you first fix the ordering in
dsa_switch_destroy()and then add the missingnetif_carrier_off()andphy_disconnect().Thanks
Andrew
Yes, you are right, I will submit a cleaned up version. I forgot the fixed phy case actually.
Thanks,
Neil