Skip to content

Commit

Permalink
OS-1988 Make ldi_ev_remove_callbacks safe to use in LDI callbacks
Browse files Browse the repository at this point in the history 
Reviewed by: Robert Mustacchi <rm@joyent.com>
  • Loading branch information
@jclulow
jclulow committed Mar 5, 2013
1 parent df8e9ae commit 0088695
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 11 deletions.
45 changes: 41 additions & 4 deletions usr/src/uts/common/os/driver_lyr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 25 additions & 7 deletions usr/src/uts/common/sys/sunldi_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 0088695

Please sign in to comment.