# HG changeset patch # User Joshua M. Clulow # Date 1362441176 0 # Node ID afe390b9f1e056d5883f5ce4aac3d266111e0287 # Parent 68927c7858895959383559f013f284729f17e388 4020 Make ldi_ev_remove_callbacks safe to use in LDI callbacks Reviewed by: Robert Mustacchi Approved by: Dan McDonald diff -r 68927c785889 -r afe390b9f1e0 usr/src/uts/common/os/driver_lyr.c --- a/usr/src/uts/common/os/driver_lyr.c Fri Sep 06 09:20:56 2013 -0700 +++ b/usr/src/uts/common/os/driver_lyr.c Mon Mar 04 23:52:56 2013 +0000 @@ -21,6 +21,9 @@ /* * Copyright (c) 1994, 2010, Oracle and/or its affiliates. All rights reserved. */ +/* + * Copyright (c) 2013, Joyent, Inc. All rights reserved. + */ /* * Layered driver support. @@ -127,6 +130,10 @@ static struct ldi_handle *ldi_handle_hash[LH_HASH_SZ]; static size_t ldi_handle_hash_count; +/* + * Use of "ldi_ev_callback_list" must be protected by ldi_ev_lock() + * and ldi_ev_unlock(). + */ static struct ldi_ev_callback_list ldi_ev_callback_list; static uint32_t ldi_ev_id_pool = 0; @@ -166,6 +173,8 @@ cv_init(&ldi_ev_callback_list.le_cv, NULL, CV_DEFAULT, NULL); ldi_ev_callback_list.le_busy = 0; ldi_ev_callback_list.le_thread = NULL; + ldi_ev_callback_list.le_walker_next = NULL; + ldi_ev_callback_list.le_walker_prev = NULL; list_create(&ldi_ev_callback_list.le_head, sizeof (ldi_ev_callback_impl_t), offsetof(ldi_ev_callback_impl_t, lec_list)); @@ -3329,8 +3338,12 @@ ret = LDI_EV_NONE; ldi_ev_lock(); + + VERIFY(ldi_ev_callback_list.le_walker_next == NULL); listp = &ldi_ev_callback_list.le_head; - for (lecp = list_head(listp); lecp; lecp = list_next(listp, lecp)) { + for (lecp = list_head(listp); lecp; lecp = + ldi_ev_callback_list.le_walker_next) { + ldi_ev_callback_list.le_walker_next = list_next(listp, lecp); /* Check if matching device */ if (!ldi_ev_device_match(lecp, dip, dev, spec_type)) @@ -3386,7 +3399,9 @@ * Undo notifies already sent */ lecp = list_prev(listp, lecp); - for (; lecp; lecp = list_prev(listp, lecp)) { + VERIFY(ldi_ev_callback_list.le_walker_prev == NULL); + for (; lecp; lecp = ldi_ev_callback_list.le_walker_prev) { + ldi_ev_callback_list.le_walker_prev = list_prev(listp, lecp); /* * Check if matching device @@ -3437,6 +3452,8 @@ } out: + ldi_ev_callback_list.le_walker_next = NULL; + ldi_ev_callback_list.le_walker_prev = NULL; ldi_ev_unlock(); if (ret == LDI_EV_NONE) { @@ -3552,8 +3569,11 @@ " event=%s", (void *)dip, ldi_result, event)); ldi_ev_lock(); + VERIFY(ldi_ev_callback_list.le_walker_next == NULL); listp = &ldi_ev_callback_list.le_head; - for (lecp = list_head(listp); lecp; lecp = list_next(listp, lecp)) { + for (lecp = list_head(listp); lecp; lecp = + ldi_ev_callback_list.le_walker_next) { + ldi_ev_callback_list.le_walker_next = list_next(listp, lecp); if (lecp->lec_finalize == NULL) { LDI_EVDBG((CE_NOTE, "ldi_invoke_finalize(): No " @@ -3604,6 +3624,7 @@ lecp->lec_finalize = NULL; } } + ldi_ev_callback_list.le_walker_next = NULL; ldi_ev_unlock(); if (found) @@ -3684,7 +3705,23 @@ for (lecp = list_head(listp); lecp; lecp = next) { next = list_next(listp, lecp); if (lecp->lec_id == id) { - ASSERT(found == NULL); + VERIFY(found == NULL); + + /* + * If there is a walk in progress, shift that walk + * along to the next element so that we can remove + * this one. This allows us to unregister an arbitrary + * number of callbacks from within a callback. + * + * See the struct definition (in sunldi_impl.h) for + * more information. + */ + if (ldi_ev_callback_list.le_walker_next == lecp) + ldi_ev_callback_list.le_walker_next = next; + if (ldi_ev_callback_list.le_walker_prev == lecp) + ldi_ev_callback_list.le_walker_prev = list_prev( + listp, ldi_ev_callback_list.le_walker_prev); + list_remove(listp, lecp); found = lecp; } diff -r 68927c785889 -r afe390b9f1e0 usr/src/uts/common/sys/sunldi_impl.h --- a/usr/src/uts/common/sys/sunldi_impl.h Fri Sep 06 09:20:56 2013 -0700 +++ b/usr/src/uts/common/sys/sunldi_impl.h Mon Mar 04 23:52:56 2013 +0000 @@ -22,12 +22,13 @@ * Copyright 2007 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ +/* + * Copyright (c) 2013, Joyent, Inc. All rights reserved. + */ #ifndef _SYS_SUNLDI_IMPL_H #define _SYS_SUNLDI_IMPL_H -#pragma ident "%Z%%M% %I% %E% SMI" - #ifdef __cplusplus extern "C" { #endif @@ -141,12 +142,29 @@ list_node_t lec_list; } ldi_ev_callback_impl_t; +/* + * Members of "struct ldi_ev_callback_list" are protected by their le_lock + * member. The struct is currently only used once, as a file-level global, + * and the locking protocol is currently implemented in ldi_ev_lock() and + * ldi_ev_unlock(). + * + * When delivering events to subscribers, ldi_invoke_notify() and + * ldi_invoke_finalize() will walk the list of callbacks: le_head. It is + * possible that an invoked callback function will need to unregister an + * arbitrary number of callbacks from this list. + * + * To enable ldi_ev_remove_callbacks() to remove elements from the list + * without breaking the walk-in-progress, we store the next element in the + * walk direction on the struct as le_walker_next and le_walker_prev. + */ struct ldi_ev_callback_list { - kmutex_t le_lock; - kcondvar_t le_cv; - int le_busy; - void *le_thread; - list_t le_head; + kmutex_t le_lock; + kcondvar_t le_cv; + int le_busy; + void *le_thread; + list_t le_head; + ldi_ev_callback_impl_t *le_walker_next; + ldi_ev_callback_impl_t *le_walker_prev; }; int ldi_invoke_notify(dev_info_t *dip, dev_t dev, int spec_type, char *event,