iflib: add iflib_deregister to help cleanup on exit
Commit message by Jake: The iflib_register function exists to allocate and setup some common structures used by both iflib_device_register and iflib_pseudo_register. There is no associated cleanup function used to undo the steps taken in this function. Both iflib_device_deregister and iflib_pseudo_deregister have some of the necessary steps scattered in their flow. However, most of the necessary cleanup is not done during the error path of iflib_device_register and iflib_pseudo_register. Some examples of missed cleanup include: the ifp pointer is not free'd during error cleanup the STATE and CTX locks are not destroyed during error cleanup the vlan event handlers are not removed during error cleanup media added to the ifmedia structure is not removed the kobject reference is never deleted Additionally, when initializing the kobject class reference counter is increased even though kobj_init already increases it. This results in the class never being free'd again because the reference count would never hit zero even after all driver instances are unloaded. To aid in proper cleanup, implement an iflib_deregister function that goes through the reverse steps taken by iflib_register. Call this function during the error cleanup for iflib_device_register and iflib_pseudo_register. Additionally call the function in the iflib_device_deregister and iflib_pseudo_deregister functions near the end of their flow. This helps reduce code duplication and ensures that proper steps are taken to cleanup allocations and references in both the regular and error cleanup flows. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Submitted by: Jacob Keller <jacob.e.keller@intel.com> Reviewed by: shurd@, erj@ MFC after: 3 days Sponsored by: Intel Corporation Differential Revision: https://reviews.freebsd.org/D21005
This commit is contained in:
parent
dc89d06976
commit
566144142e
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=351152
@ -702,6 +702,7 @@ static void iflib_altq_if_start(if_t ifp);
|
|||||||
static int iflib_altq_if_transmit(if_t ifp, struct mbuf *m);
|
static int iflib_altq_if_transmit(if_t ifp, struct mbuf *m);
|
||||||
#endif
|
#endif
|
||||||
static int iflib_register(if_ctx_t);
|
static int iflib_register(if_ctx_t);
|
||||||
|
static void iflib_deregister(if_ctx_t);
|
||||||
static void iflib_init_locked(if_ctx_t ctx);
|
static void iflib_init_locked(if_ctx_t ctx);
|
||||||
static void iflib_add_device_sysctl_pre(if_ctx_t ctx);
|
static void iflib_add_device_sysctl_pre(if_ctx_t ctx);
|
||||||
static void iflib_add_device_sysctl_post(if_ctx_t ctx);
|
static void iflib_add_device_sysctl_post(if_ctx_t ctx);
|
||||||
@ -4790,6 +4791,7 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
|
|||||||
IFDI_DETACH(ctx);
|
IFDI_DETACH(ctx);
|
||||||
fail_unlock:
|
fail_unlock:
|
||||||
CTX_UNLOCK(ctx);
|
CTX_UNLOCK(ctx);
|
||||||
|
iflib_deregister(ctx);
|
||||||
fail_ctx_free:
|
fail_ctx_free:
|
||||||
device_set_softc(ctx->ifc_dev, NULL);
|
device_set_softc(ctx->ifc_dev, NULL);
|
||||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
||||||
@ -4983,6 +4985,7 @@ iflib_pseudo_register(device_t dev, if_shared_ctx_t sctx, if_ctx_t *ctxp,
|
|||||||
IFDI_DETACH(ctx);
|
IFDI_DETACH(ctx);
|
||||||
fail_unlock:
|
fail_unlock:
|
||||||
CTX_UNLOCK(ctx);
|
CTX_UNLOCK(ctx);
|
||||||
|
iflib_deregister(ctx);
|
||||||
fail_ctx_free:
|
fail_ctx_free:
|
||||||
free(ctx->ifc_softc, M_IFLIB);
|
free(ctx->ifc_softc, M_IFLIB);
|
||||||
free(ctx, M_IFLIB);
|
free(ctx, M_IFLIB);
|
||||||
@ -4999,15 +5002,7 @@ iflib_pseudo_deregister(if_ctx_t ctx)
|
|||||||
struct taskqgroup *tqg;
|
struct taskqgroup *tqg;
|
||||||
iflib_fl_t fl;
|
iflib_fl_t fl;
|
||||||
|
|
||||||
/* Unregister VLAN events */
|
|
||||||
if (ctx->ifc_vlan_attach_event != NULL)
|
|
||||||
EVENTHANDLER_DEREGISTER(vlan_config, ctx->ifc_vlan_attach_event);
|
|
||||||
if (ctx->ifc_vlan_detach_event != NULL)
|
|
||||||
EVENTHANDLER_DEREGISTER(vlan_unconfig, ctx->ifc_vlan_detach_event);
|
|
||||||
|
|
||||||
ether_ifdetach(ifp);
|
ether_ifdetach(ifp);
|
||||||
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/
|
|
||||||
CTX_LOCK_DESTROY(ctx);
|
|
||||||
/* XXX drain any dependent tasks */
|
/* XXX drain any dependent tasks */
|
||||||
tqg = qgroup_if_io_tqg;
|
tqg = qgroup_if_io_tqg;
|
||||||
for (txq = ctx->ifc_txqs, i = 0; i < NTXQSETS(ctx); i++, txq++) {
|
for (txq = ctx->ifc_txqs, i = 0; i < NTXQSETS(ctx); i++, txq++) {
|
||||||
@ -5028,10 +5023,11 @@ iflib_pseudo_deregister(if_ctx_t ctx)
|
|||||||
if (ctx->ifc_vflr_task.gt_uniq != NULL)
|
if (ctx->ifc_vflr_task.gt_uniq != NULL)
|
||||||
taskqgroup_detach(tqg, &ctx->ifc_vflr_task);
|
taskqgroup_detach(tqg, &ctx->ifc_vflr_task);
|
||||||
|
|
||||||
if_free(ifp);
|
|
||||||
|
|
||||||
iflib_tx_structures_free(ctx);
|
iflib_tx_structures_free(ctx);
|
||||||
iflib_rx_structures_free(ctx);
|
iflib_rx_structures_free(ctx);
|
||||||
|
|
||||||
|
iflib_deregister(ctx);
|
||||||
|
|
||||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
||||||
free(ctx->ifc_softc, M_IFLIB);
|
free(ctx->ifc_softc, M_IFLIB);
|
||||||
free(ctx, M_IFLIB);
|
free(ctx, M_IFLIB);
|
||||||
@ -5118,19 +5114,19 @@ iflib_device_deregister(if_ctx_t ctx)
|
|||||||
CTX_UNLOCK(ctx);
|
CTX_UNLOCK(ctx);
|
||||||
|
|
||||||
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/
|
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/
|
||||||
CTX_LOCK_DESTROY(ctx);
|
|
||||||
device_set_softc(ctx->ifc_dev, NULL);
|
|
||||||
iflib_free_intr_mem(ctx);
|
iflib_free_intr_mem(ctx);
|
||||||
|
|
||||||
bus_generic_detach(dev);
|
bus_generic_detach(dev);
|
||||||
if_free(ifp);
|
|
||||||
|
|
||||||
iflib_tx_structures_free(ctx);
|
iflib_tx_structures_free(ctx);
|
||||||
iflib_rx_structures_free(ctx);
|
iflib_rx_structures_free(ctx);
|
||||||
|
|
||||||
|
iflib_deregister(ctx);
|
||||||
|
|
||||||
|
device_set_softc(ctx->ifc_dev, NULL);
|
||||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
if (ctx->ifc_flags & IFC_SC_ALLOCATED)
|
||||||
free(ctx->ifc_softc, M_IFLIB);
|
free(ctx->ifc_softc, M_IFLIB);
|
||||||
unref_ctx_core_offset(ctx);
|
unref_ctx_core_offset(ctx);
|
||||||
STATE_LOCK_DESTROY(ctx);
|
|
||||||
free(ctx, M_IFLIB);
|
free(ctx, M_IFLIB);
|
||||||
return (0);
|
return (0);
|
||||||
}
|
}
|
||||||
@ -5379,6 +5375,36 @@ iflib_register(if_ctx_t ctx)
|
|||||||
return (0);
|
return (0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void
|
||||||
|
iflib_deregister(if_ctx_t ctx)
|
||||||
|
{
|
||||||
|
if_t ifp = ctx->ifc_ifp;
|
||||||
|
|
||||||
|
/* Remove all media */
|
||||||
|
ifmedia_removeall(&ctx->ifc_media);
|
||||||
|
|
||||||
|
/* Unregister VLAN events */
|
||||||
|
if (ctx->ifc_vlan_attach_event != NULL) {
|
||||||
|
EVENTHANDLER_DEREGISTER(vlan_config, ctx->ifc_vlan_attach_event);
|
||||||
|
ctx->ifc_vlan_attach_event = NULL;
|
||||||
|
}
|
||||||
|
if (ctx->ifc_vlan_detach_event != NULL) {
|
||||||
|
EVENTHANDLER_DEREGISTER(vlan_unconfig, ctx->ifc_vlan_detach_event);
|
||||||
|
ctx->ifc_vlan_detach_event = NULL;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Release kobject reference */
|
||||||
|
kobj_delete((kobj_t) ctx, NULL);
|
||||||
|
|
||||||
|
/* Free the ifnet structure */
|
||||||
|
if_free(ifp);
|
||||||
|
|
||||||
|
STATE_LOCK_DESTROY(ctx);
|
||||||
|
|
||||||
|
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/
|
||||||
|
CTX_LOCK_DESTROY(ctx);
|
||||||
|
}
|
||||||
|
|
||||||
static int
|
static int
|
||||||
iflib_queues_alloc(if_ctx_t ctx)
|
iflib_queues_alloc(if_ctx_t ctx)
|
||||||
{
|
{
|
||||||
|
Loading…
Reference in New Issue
Block a user