From eb53c55f360774899e5c43d07705abccf97460d3 Mon Sep 17 00:00:00 2001 From: julian Date: Thu, 11 Jan 2001 22:22:52 +0000 Subject: Make hook deletion SMP safe. --- sys/netgraph/ng_base.c | 140 +++++++++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 52 deletions(-) (limited to 'sys/netgraph') diff --git a/sys/netgraph/ng_base.c b/sys/netgraph/ng_base.c index 1d80233..cdf45e7 100644 --- a/sys/netgraph/ng_base.c +++ b/sys/netgraph/ng_base.c @@ -181,7 +181,6 @@ static node_p ng_ID2noderef(ng_ID_t ID); /* imported , these used to be externally visible, some may go back */ int ng_bypass(hook_p hook1, hook_p hook2); -void ng_cutlinks(node_p node); int ng_con_nodes(node_p node, const char *name, node_p node2, const char *name2); void ng_destroy_hook(hook_p hook); @@ -647,14 +646,23 @@ ng_make_node_common(struct ng_type *type, node_p *nodepp) * no type-specific method. * * We can only be called form a shutdown message, so we know we have - * a writer lock, and therefore exclusive access. + * a writer lock, and therefore exclusive access. It also means + * that we should not be on the work queue, but we check anyhow. * * Persistent node types must have a type-specific method which - * Allocates a new node. This one is irretrievably going away. + * Allocates a new node in which case, this one is irretrievably going away, + * or cleans up anything it needs, and just makes the node valid again, + * in which case we allow the node to survive. + * + * XXX We need to think of how to tell a persistant node that we + * REALLY need to go away because the hardware has gone or we + * are rebooting.... etc. */ void ng_rmnode(node_p node) { + hook_p hook; + /* Check if it's already shutting down */ if ((node->nd_flags & NG_CLOSING) != 0) return; @@ -670,7 +678,9 @@ ng_rmnode(node_p node) */ node->nd_flags |= NG_INVALID|NG_CLOSING; - ng_cutlinks(node); + /* Notify all remaining connected nodes to disconnect */ + while ((hook = LIST_FIRST(&node->nd_hooks)) != NULL) + ng_destroy_hook(hook); /* * Drain the input queue forceably. @@ -717,30 +727,6 @@ ng_rmnode(node_p node) } /* - * Called by the destructor to remove any STANDARD external references - * May one day have it's own message to call it.. - */ -void -ng_cutlinks(node_p node) -{ - hook_p hook; - - /* Make sure that this is set to stop infinite loops */ - node->nd_flags |= NG_INVALID; - - /* - * Drain the input queue forceably. - * We also do this in ng_rmnode - * to make sure we get all code paths. - */ - ng_flush_input_queue(&node->nd_input_queue); - - /* Notify all remaining connected nodes to disconnect */ - while ((hook = LIST_FIRST(&node->nd_hooks)) != NULL) - ng_destroy_hook(hook); -} - -/* * Remove a reference to the node, possibly the last */ void @@ -915,7 +901,8 @@ ng_unname(node_p node) /************************************************************************ Hook routines Names are not optional. Hooks are always connected, except for a - brief moment within these routines. + brief moment within these routines. On invalidation or during creation + they are connected to the 'dead' hook. ************************************************************************/ /* @@ -940,6 +927,7 @@ ng_unref_hook(hook_p hook) /* * Add an unconnected hook to a node. Only used internally. + * Assumes node is locked. (XXX not yet true ) */ static int ng_add_hook(node_p node, const char *name, hook_p *hookp) @@ -963,12 +951,19 @@ ng_add_hook(node_p node, const char *name, hook_p *hookp) TRAP_ERROR; return (ENOMEM); } - hook->hk_refs = 1; + hook->hk_refs = 1; /* add a reference for us to return */ hook->hk_flags = HK_INVALID; + hook->hk_peer = &ng_deadhook; /* start off this way */ hook->hk_node = node; NG_NODE_REF(node); /* each hook counts as a reference */ - /* Check if the node type code has something to say about it */ + /* Set hook name */ + strncpy(NG_HOOK_NAME(hook), name, NG_HOOKLEN); + + /* + * Check if the node type code has something to say about it + * If it fails, the unref of the hook will also unref the node. + */ if (node->nd_type->newhook != NULL) { if ((error = (*node->nd_type->newhook)(node, hook, name))) { NG_HOOK_UNREF(hook); /* this frees the hook */ @@ -978,20 +973,21 @@ ng_add_hook(node_p node, const char *name, hook_p *hookp) /* * The 'type' agrees so far, so go ahead and link it in. * We'll ask again later when we actually connect the hooks. - * The reference we have is for this linkage. */ LIST_INSERT_HEAD(&node->nd_hooks, hook, hk_hooks); node->nd_numhooks++; + NG_HOOK_REF(hook); /* one for the node */ - /* Set hook name */ - strncpy(NG_HOOK_NAME(hook), name, NG_HOOKLEN); if (hookp) *hookp = hook; - return (error); + return (0); } /* * Connect a pair of hooks. Only used internally. + * Assume whoever called us has the nodes locked + * and is holding refs on the hooks. + * XXX This may have to change to be SMP safe. */ static int ng_connect(hook_p hook1, hook_p hook2) @@ -1001,7 +997,11 @@ ng_connect(hook_p hook1, hook_p hook2) hook1->hk_peer = hook2; hook2->hk_peer = hook1; - /* Give each node the opportunity to veto the impending connection */ + /* Each hook is referenced by the other */ + NG_HOOK_REF(hook1); + NG_HOOK_REF(hook2); + + /* Give each node the opportunity to veto the pending connection */ if (hook1->hk_node->nd_type->connect) { if ((error = (*hook1->hk_node->nd_type->connect) (hook1))) { ng_destroy_hook(hook1); /* also zaps hook2 */ @@ -1014,6 +1014,8 @@ ng_connect(hook_p hook1, hook_p hook2) return (error); } } + + /* As a last act, allow the hooks to be used */ hook1->hk_flags &= ~HK_INVALID; hook2->hk_flags &= ~HK_INVALID; return (0); @@ -1024,6 +1026,7 @@ ng_connect(hook_p hook1, hook_p hook2) * * Node types may supply their own optimized routines for finding * hooks. If none is supplied, we just do a linear search. + * XXX Possibly we should add a reference to the hook? */ hook_p ng_findhook(node_p node, const char *name) @@ -1045,7 +1048,15 @@ ng_findhook(node_p node, const char *name) * * As hooks are always attached, this really destroys two hooks. * The one given, and the one attached to it. Disconnect the hooks - * from each other first. + * from each other first. We reconnect the peer hook to the 'dead' + * hook so that it can still exist after we depart. We then + * send the peer its own destroy message. This ensures that we only + * interact with the peer's structures when it is locked processing that + * message. We hold a reference to the peer hook so we are guaranteed that + * the peer hook and node are still going to exist until + * we are finished there as the hook holds a ref on the node. + * We run this same code again on the peer hook, but that time it is already + * attached to the 'dead' hook. */ void ng_destroy_hook(hook_p hook) @@ -1053,21 +1064,26 @@ ng_destroy_hook(hook_p hook) hook_p peer = NG_HOOK_PEER(hook); hook->hk_flags |= HK_INVALID; /* as soon as possible */ - if (peer) { - peer->hk_flags |= HK_INVALID; /* as soon as possible */ - hook->hk_peer = NULL; - peer->hk_peer = NULL; - ng_disconnect_hook(peer); + if (peer && (peer != &ng_deadhook)) { + /* + * Set the peer to point to ng_deadhook + * from this moment on we are effectively independent it. + * send it an rmhook message of it's own. + */ + peer->hk_peer = &ng_deadhook; /* They no longer know us */ + hook->hk_peer = &ng_deadhook; /* Nor us, them */ + ng_rmhook_self(peer); /* Give it a surprise */ } ng_disconnect_hook(hook); + NG_HOOK_UNREF(hook); /* account for peer link */ } /* * Notify the node of the hook's demise. This may result in more actions * (e.g. shutdown) but we don't do that ourselves and don't know what * happens there. If there is no appropriate handler, then just remove it - * (and decrement the reference count of it's node which in turn might - * make something happen). + * and decrement the reference count. When it's freed it will decrement + * the reference count of it's node which in turn might make something happen. */ static void ng_disconnect_hook(hook_p hook) @@ -1087,7 +1103,7 @@ ng_disconnect_hook(hook_p hook) */ (*node->nd_type->disconnect) (hook); } - NG_HOOK_UNREF(hook); + NG_HOOK_UNREF(hook); /* Account for linkage to node */ } /* @@ -1104,9 +1120,10 @@ ng_bypass(hook_p hook1, hook_p hook2) hook1->hk_peer->hk_peer = hook2->hk_peer; hook2->hk_peer->hk_peer = hook1->hk_peer; + hook1->hk_peer = &ng_deadhook; + hook2->hk_peer = &ng_deadhook; + /* XXX If we ever cache methods on hooks update them as well */ - hook1->hk_peer = NULL; - hook2->hk_peer = NULL; ng_destroy_hook(hook1); ng_destroy_hook(hook2); return (0); @@ -1167,6 +1184,9 @@ ng_findtype(const char *typename) /* * Make a peer and connect. The order is arranged to minimise * the work needed to back out in case of error. + * We assume that the local node is locked. + * The new node probably doesn't need a lock until + * it has a hook, but we should think about it a bit more. */ int ng_mkpeer(node_p node, const char *name, const char *name2, char *type) @@ -1176,15 +1196,17 @@ ng_mkpeer(node_p node, const char *name, const char *name2, char *type) hook_p hook2; int error; - if ((error = ng_add_hook(node, name, &hook))) + if ((error = ng_add_hook(node, name, &hook))) /* gives us a ref */ return (error); if ((error = ng_make_node(type, &node2))) { ng_destroy_hook(hook); + NG_HOOK_UNREF(hook); return (error); } if ((error = ng_add_hook(node2, name2, &hook2))) { ng_rmnode(node2); ng_destroy_hook(hook); + NG_HOOK_UNREF(hook); return (error); } @@ -1192,13 +1214,19 @@ ng_mkpeer(node_p node, const char *name, const char *name2, char *type) * Actually link the two hooks together.. on failure they are * destroyed so we don't have to do that here. */ - if ((error = ng_connect(hook, hook2))) + if ((error = ng_connect(hook, hook2))) ng_rmnode(node2); + /* + * drop the references we were holding on the two hooks. + */ + NG_HOOK_UNREF(hook); + NG_HOOK_UNREF(hook2); return (error); } /* * Connect two nodes using the specified hooks + * XXX need to think of a SMP safe way of doing this. */ int ng_con_nodes(node_p node, const char *name, node_p node2, const char *name2) @@ -1207,13 +1235,21 @@ ng_con_nodes(node_p node, const char *name, node_p node2, const char *name2) hook_p hook; hook_p hook2; - if ((error = ng_add_hook(node, name, &hook))) + if ((error = ng_add_hook(node, name, &hook))) /* gives us a ref */ return (error); - if ((error = ng_add_hook(node2, name2, &hook2))) { + if ((error = ng_add_hook(node2, name2, &hook2))) { /* we get a ref */ ng_destroy_hook(hook); + NG_HOOK_UNREF(hook); return (error); } - return (ng_connect(hook, hook2)); + error = ng_connect(hook, hook2); + + /* + * drop the references we were holding on the two hooks. + */ + NG_HOOK_UNREF(hook); + NG_HOOK_UNREF(hook2); + return (error); } /************************************************************************ Utility routines to send self messages -- cgit v1.1