Date: Tue, 27 Jan 2015 00:00:19 +0000 From: Sreekanth Rupavatharam <rupavath@juniper.net> To: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org> Subject: Double cleanup in igb_attach Message-ID: <D0EC151C.1A7B1%rupavath@juniper.net>
next in thread | raw e-mail | index | archive | help
Apologies if this is not the right forum. In igb_attach function, we have this code.
err_late:
igb_detach(dev);
igb_free_transmit_structures(adapter);
igb_free_receive_structures(adapter);
igb_release_hw_control(adapter);
err_pci:
igb_free_pci_resources(adapter);
if (adapter->ifp != NULL)
if_free(adapter->ifp);
free(adapter->mta, M_DEVBUF);
IGB_CORE_LOCK_DESTROY(adapter);
The problem is that igb_detach also does the same cleanup in it’s body. Only exception is this case where it just returns EBUSY
/* Make sure VLANS are not using driver */
if (if_vlantrunkinuse(ifp)) {
device_printf(dev,"Vlan in use, detach first\n");
return (EBUSY);
}
I think the code in igb_attach should be changed to free up resources only if the igb_detach returns an error. Here’s the patch for it.
Index: if_igb.c
===================================================================
--- if_igb.c (revision 298025)
+++ if_igb.c (working copy)
@@ -723,7 +723,8 @@ igb_attach(device_t dev)
return (0);
err_late:
- igb_detach(dev);
+ if(igb_detach(dev) == 0) /* igb_detach did the cleanup */
+ return;
igb_free_transmit_structures(adapter);
Can anyone comment on it and tell me if my understanding is incorrect?
-- Thanks,
Sreekanth
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D0EC151C.1A7B1%rupavath>