4020 Make ldi_ev_remove_callbacks safe to use in LDI callbacks

Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Dan McDonald <danmcd@nexenta.com>

2 files changed, 66 insertions, 11 deletions

diff --git a/usr/src/uts/common/os/driver_lyr.c b/usr/src/uts/common/os/driver_lyr.c
index 8386a1efd8..e59acc4370 100644
--- a/usr/src/uts/common/os/driver_lyr.c
+++ b/usr/src/uts/common/os/driver_lyr.c
@@ -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 kmutex_t			ldi_handle_hash_lock[LH_HASH_SZ];
 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 @@ ldi_init(void)
 	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 @@ ldi_invoke_notify(dev_info_t *dip, dev_t dev, int spec_type, char *event,
 
 	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 @@ ldi_invoke_notify(dev_info_t *dip, dev_t dev, int spec_type, char *event,
 	 * 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 @@ ldi_invoke_notify(dev_info_t *dip, dev_t dev, int spec_type, char *event,
 	}
 
 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 @@ ldi_invoke_finalize(dev_info_t *dip, dev_t dev, int spec_type, char *event,
 	    " 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 @@ ldi_invoke_finalize(dev_info_t *dip, dev_t dev, int spec_type, char *event,
 			lecp->lec_finalize = NULL;
 		}
 	}
+	ldi_ev_callback_list.le_walker_next = NULL;
 	ldi_ev_unlock();
 
 	if (found)
@@ -3684,7 +3705,23 @@ ldi_ev_remove_callbacks(ldi_callback_id_t id)
 	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 --git a/usr/src/uts/common/sys/sunldi_impl.h b/usr/src/uts/common/sys/sunldi_impl.h
index 1156fe2c41..e9fdb431bf 100644
--- a/usr/src/uts/common/sys/sunldi_impl.h
+++ b/usr/src/uts/common/sys/sunldi_impl.h
@@ -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 @@ typedef struct ldi_ev_callback_impl {
 	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,