4020 Make ldi_ev_remove_callbacks safe to use in LDI callbacks
Reviewed by: Robert Mustacchi <
[email protected]>
Approved by: Dan McDonald <
[email protected]>
--- 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;
}
--- 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,