Print this page
OS-XXXX netstack_find_by_stackid() drops-and-reacquires
OS-5423 deadlock between netstack teardown and kstat read

*** 35,44 **** --- 35,45 ---- #include <sys/debug.h> #include <sys/sdt.h> #include <sys/mutex.h> #include <sys/bitmap.h> #include <sys/atomic.h> + #include <sys/sunddi.h> #include <sys/kobj.h> #include <sys/disp.h> #include <vm/seg_kmem.h> #include <sys/zone.h> #include <sys/netstack.h>
*** 120,135 **** --- 121,143 ---- static boolean_t netstack_apply_destroy(kmutex_t *, netstack_t *, int); static boolean_t wait_for_zone_creator(netstack_t *, kmutex_t *); static boolean_t wait_for_nms_inprogress(netstack_t *, nm_state_t *, kmutex_t *); + static void netstack_hold_locked(netstack_t *); + static void netstack_reap_work(netstack_t *, boolean_t); + ksema_t netstack_reap_limiter; + void netstack_init(void) { mutex_init(&netstack_g_lock, NULL, MUTEX_DEFAULT, NULL); mutex_init(&netstack_shared_lock, NULL, MUTEX_DEFAULT, NULL); + /* XXX KEBE SAYS hard-coded constant needs to be fixed. */ + sema_init(&netstack_reap_limiter, 1024, NULL, SEMA_DRIVER, NULL); + netstack_initialized = 1; /* * We want to be informed each time a zone is created or * destroyed in the kernel, so we can maintain the
*** 1028,1039 **** mutex_enter(&netstack_g_lock); for (ns = netstack_head; ns != NULL; ns = ns->netstack_next) { mutex_enter(&ns->netstack_lock); if (ns->netstack_stackid == stackid && !(ns->netstack_flags & (NSF_UNINIT|NSF_CLOSING))) { mutex_exit(&ns->netstack_lock); - netstack_hold(ns); mutex_exit(&netstack_g_lock); return (ns); } mutex_exit(&ns->netstack_lock); } --- 1036,1047 ---- mutex_enter(&netstack_g_lock); for (ns = netstack_head; ns != NULL; ns = ns->netstack_next) { mutex_enter(&ns->netstack_lock); if (ns->netstack_stackid == stackid && !(ns->netstack_flags & (NSF_UNINIT|NSF_CLOSING))) { + netstack_hold_locked(ns); mutex_exit(&ns->netstack_lock); mutex_exit(&netstack_g_lock); return (ns); } mutex_exit(&ns->netstack_lock); }
*** 1059,1092 **** mutex_exit(&netstack_g_lock); return (rval); } ! void ! netstack_rele(netstack_t *ns) { netstack_t **nsp; boolean_t found; - int refcnt, numzones; int i; - mutex_enter(&ns->netstack_lock); - ASSERT(ns->netstack_refcnt > 0); - ns->netstack_refcnt--; /* - * As we drop the lock additional netstack_rele()s can come in - * and decrement the refcnt to zero and free the netstack_t. - * Store pointers in local variables and if we were not the last - * then don't reference the netstack_t after that. - */ - refcnt = ns->netstack_refcnt; - numzones = ns->netstack_numzones; - DTRACE_PROBE1(netstack__dec__ref, netstack_t *, ns); - mutex_exit(&ns->netstack_lock); - - if (refcnt == 0 && numzones == 0) { - /* * Time to call the destroy functions and free up * the structure */ netstack_stack_inactive(ns); --- 1067,1099 ---- mutex_exit(&netstack_g_lock); return (rval); } ! ! static void ! netstack_reap(void *arg) { + /* Indicate we took a semaphore to get here. */ + netstack_reap_work((netstack_t *)arg, B_TRUE); + } + + static void + netstack_reap_intr(void *arg) + { + /* Indicate we did NOT TAKE a semaphore to get here. */ + netstack_reap_work((netstack_t *)arg, B_FALSE); + } + + static void + netstack_reap_work(netstack_t *ns, boolean_t semaphore_signal) + { netstack_t **nsp; boolean_t found; int i; /* * Time to call the destroy functions and free up * the structure */ netstack_stack_inactive(ns);
*** 1121,1143 **** cv_destroy(&nms->nms_cv); } mutex_destroy(&ns->netstack_lock); cv_destroy(&ns->netstack_cv); kmem_free(ns, sizeof (*ns)); ! } } void ! netstack_hold(netstack_t *ns) { mutex_enter(&ns->netstack_lock); - ns->netstack_refcnt++; ASSERT(ns->netstack_refcnt > 0); mutex_exit(&ns->netstack_lock); DTRACE_PROBE1(netstack__inc__ref, netstack_t *, ns); } /* * To support kstat_create_netstack() using kstat_zone_add we need * to track both * - all zoneids that use the global/shared stack * - all kstats that have been added for the shared stack --- 1128,1220 ---- cv_destroy(&nms->nms_cv); } mutex_destroy(&ns->netstack_lock); cv_destroy(&ns->netstack_cv); kmem_free(ns, sizeof (*ns)); ! /* Allow another reap to be scheduled. */ ! if (semaphore_signal) ! sema_v(&netstack_reap_limiter); } void ! netstack_rele(netstack_t *ns) { + int refcnt, numzones; + mutex_enter(&ns->netstack_lock); ASSERT(ns->netstack_refcnt > 0); + ns->netstack_refcnt--; + /* + * As we drop the lock additional netstack_rele()s can come in + * and decrement the refcnt to zero and free the netstack_t. + * Store pointers in local variables and if we were not the last + * then don't reference the netstack_t after that. + */ + refcnt = ns->netstack_refcnt; + numzones = ns->netstack_numzones; + DTRACE_PROBE1(netstack__dec__ref, netstack_t *, ns); mutex_exit(&ns->netstack_lock); + + if (refcnt == 0 && numzones == 0) { + boolean_t is_not_intr = !servicing_interrupt(); + + /* + * Because there are possibilities of kstats being held by + * callers, which would then be immediately freed, but held up + * due to kstat's odd reference model recording the thread, we + * choose to schedule the actual deletion of this netstack as + * a deferred task on the system taskq. This way, any + * store-the-thread-pointer semantics won't trip over + * themselves. + * + * On the off chance this is called in interrupt context, we + * cannot use the semaphore to enforce rate-limiting. + */ + if (is_not_intr && sema_tryp(&netstack_reap_limiter) == 0) { + /* + * XXX KEBE SAYS inidicate we're slamming against + * a limit. + */ + hrtime_t measurement = gethrtime(); + + sema_p(&netstack_reap_limiter); + /* Caputre delay in ns. */ + DTRACE_PROBE1(netstack__reap__rate__limited, + hrtime_t *, gethrtime() - measurement); + } + + if (taskq_dispatch(system_taskq, + is_not_intr ? netstack_reap : netstack_reap_intr, ns, + TQ_NOSLEEP) == NULL) { + /* + * Well shoot, why can't we taskq_dispatch? + * Take our chances with a direct call. + */ + DTRACE_PROBE1(netstack__reap__taskq__fail, + netstack_t *, ns); + netstack_reap_work(ns, is_not_intr); + } + } + } + + static void + netstack_hold_locked(netstack_t *ns) + { + ASSERT(MUTEX_HELD(&ns->netstack_lock)); + ns->netstack_refcnt++; + ASSERT(ns->netstack_refcnt > 0); DTRACE_PROBE1(netstack__inc__ref, netstack_t *, ns); } + void + netstack_hold(netstack_t *ns) + { + mutex_enter(&ns->netstack_lock); + netstack_hold_locked(ns); + mutex_exit(&ns->netstack_lock); + } + /* * To support kstat_create_netstack() using kstat_zone_add we need * to track both * - all zoneids that use the global/shared stack * - all kstats that have been added for the shared stack