svn commit: r251770 - in head/sys: kern sys
Lawrence Stewart
lstewart at FreeBSD.org
Sat Jun 15 04:03:41 UTC 2013
Author: lstewart
Date: Sat Jun 15 04:03:40 2013
New Revision: 251770
URL: http://svnweb.freebsd.org/changeset/base/251770
Log:
Internalise handling of virtualised hook points inside
hhook_{add|remove}_hook_lookup() so that khelp (and other potential API
consumers) do not have to care when they attempt to (un)hook a particular hook
point identified by id and type.
Reviewed by: scottl
MFC after: 1 week
Modified:
head/sys/kern/kern_hhook.c
head/sys/kern/kern_khelp.c
head/sys/sys/hhook.h
Modified: head/sys/kern/kern_hhook.c
==============================================================================
--- head/sys/kern/kern_hhook.c Sat Jun 15 03:55:04 2013 (r251769)
+++ head/sys/kern/kern_hhook.c Sat Jun 15 04:03:40 2013 (r251770)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 2010 Lawrence Stewart <lstewart at freebsd.org>
+ * Copyright (c) 2010,2013 Lawrence Stewart <lstewart at freebsd.org>
* Copyright (c) 2010 The FreeBSD Foundation
* All rights reserved.
*
@@ -69,6 +69,9 @@ static struct mtx hhook_head_list_lock;
MTX_SYSINIT(hhookheadlistlock, &hhook_head_list_lock, "hhook_head list lock",
MTX_DEF);
+/* Protected by hhook_head_list_lock. */
+static uint32_t n_hhookheads;
+
/* Private function prototypes. */
static void hhook_head_destroy(struct hhook_head *hhh);
@@ -165,21 +168,71 @@ hhook_add_hook(struct hhook_head *hhh, s
}
/*
- * Lookup a helper hook point and register a new helper hook function with it.
+ * Register a helper hook function with a helper hook point (including all
+ * virtual instances of the hook point if it is virtualised).
+ *
+ * The logic is unfortunately far more complex than for
+ * hhook_remove_hook_lookup() because hhook_add_hook() can call malloc() with
+ * M_WAITOK and thus we cannot call hhook_add_hook() with the
+ * hhook_head_list_lock held.
+ *
+ * The logic assembles an array of hhook_head structs that correspond to the
+ * helper hook point being hooked and bumps the refcount on each (all done with
+ * the hhook_head_list_lock held). The hhook_head_list_lock is then dropped, and
+ * hhook_add_hook() is called and the refcount dropped for each hhook_head
+ * struct in the array.
*/
int
hhook_add_hook_lookup(struct hookinfo *hki, uint32_t flags)
{
- struct hhook_head *hhh;
- int error;
+ struct hhook_head **heads_to_hook, *hhh;
+ int error, i, n_heads_to_hook;
- hhh = hhook_head_get(hki->hook_type, hki->hook_id);
+tryagain:
+ error = i = 0;
+ /*
+ * Accessing n_hhookheads without hhook_head_list_lock held opens up a
+ * race with hhook_head_register() which we are unlikely to lose, but
+ * nonetheless have to cope with - hence the complex goto logic.
+ */
+ n_heads_to_hook = n_hhookheads;
+ heads_to_hook = malloc(n_heads_to_hook * sizeof(struct hhook_head *),
+ M_HHOOK, flags & HHOOK_WAITOK ? M_WAITOK : M_NOWAIT);
+ if (heads_to_hook == NULL)
+ return (ENOMEM);
- if (hhh == NULL)
- return (ENOENT);
+ HHHLIST_LOCK();
+ LIST_FOREACH(hhh, &hhook_head_list, hhh_next) {
+ if (hhh->hhh_type == hki->hook_type &&
+ hhh->hhh_id == hki->hook_id) {
+ if (i < n_heads_to_hook) {
+ heads_to_hook[i] = hhh;
+ refcount_acquire(&heads_to_hook[i]->hhh_refcount);
+ i++;
+ } else {
+ /*
+ * We raced with hhook_head_register() which
+ * inserted a hhook_head that we need to hook
+ * but did not malloc space for. Abort this run
+ * and try again.
+ */
+ for (i--; i >= 0; i--)
+ refcount_release(&heads_to_hook[i]->hhh_refcount);
+ free(heads_to_hook, M_HHOOK);
+ HHHLIST_UNLOCK();
+ goto tryagain;
+ }
+ }
+ }
+ HHHLIST_UNLOCK();
- error = hhook_add_hook(hhh, hki, flags);
- hhook_head_release(hhh);
+ for (i--; i >= 0; i--) {
+ if (!error)
+ error = hhook_add_hook(heads_to_hook[i], hki, flags);
+ refcount_release(&heads_to_hook[i]->hhh_refcount);
+ }
+
+ free(heads_to_hook, M_HHOOK);
return (error);
}
@@ -211,20 +264,21 @@ hhook_remove_hook(struct hhook_head *hhh
}
/*
- * Lookup a helper hook point and remove a helper hook function from it.
+ * Remove a helper hook function from a helper hook point (including all
+ * virtual instances of the hook point if it is virtualised).
*/
int
hhook_remove_hook_lookup(struct hookinfo *hki)
{
struct hhook_head *hhh;
- hhh = hhook_head_get(hki->hook_type, hki->hook_id);
-
- if (hhh == NULL)
- return (ENOENT);
-
- hhook_remove_hook(hhh, hki);
- hhook_head_release(hhh);
+ HHHLIST_LOCK();
+ LIST_FOREACH(hhh, &hhook_head_list, hhh_next) {
+ if (hhh->hhh_type == hki->hook_type &&
+ hhh->hhh_id == hki->hook_id)
+ hhook_remove_hook(hhh, hki);
+ }
+ HHHLIST_UNLOCK();
return (0);
}
@@ -274,6 +328,7 @@ hhook_head_register(int32_t hhook_type,
#endif
}
LIST_INSERT_HEAD(&hhook_head_list, tmphhh, hhh_next);
+ n_hhookheads++;
HHHLIST_UNLOCK();
return (0);
@@ -285,6 +340,7 @@ hhook_head_destroy(struct hhook_head *hh
struct hhook *tmp, *tmp2;
HHHLIST_LOCK_ASSERT();
+ KASSERT(n_hhookheads > 0, ("n_hhookheads should be > 0"));
LIST_REMOVE(hhh, hhh_next);
#ifdef VIMAGE
@@ -297,6 +353,7 @@ hhook_head_destroy(struct hhook_head *hh
HHH_WUNLOCK(hhh);
HHH_LOCK_DESTROY(hhh);
free(hhh, M_HHOOK);
+ n_hhookheads--;
}
/*
Modified: head/sys/kern/kern_khelp.c
==============================================================================
--- head/sys/kern/kern_khelp.c Sat Jun 15 03:55:04 2013 (r251769)
+++ head/sys/kern/kern_khelp.c Sat Jun 15 04:03:40 2013 (r251770)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 2010 Lawrence Stewart <lstewart at freebsd.org>
+ * Copyright (c) 2010,2013 Lawrence Stewart <lstewart at freebsd.org>
* Copyright (c) 2010 The FreeBSD Foundation
* All rights reserved.
*
@@ -84,12 +84,13 @@ khelp_register_helper(struct helper *h)
for (i = 0; i < h->h_nhooks && !error; i++) {
/* We don't require the module to assign hook_helper. */
h->h_hooks[i].hook_helper = h;
- error = khelp_add_hhook(&h->h_hooks[i], HHOOK_NOWAIT);
+ error = hhook_add_hook_lookup(&h->h_hooks[i],
+ HHOOK_WAITOK);
}
if (error) {
for (i--; i >= 0; i--)
- khelp_remove_hhook(&h->h_hooks[i]);
+ hhook_remove_hook_lookup(&h->h_hooks[i]);
osd_deregister(OSD_KHELP, h->h_id);
}
@@ -144,7 +145,7 @@ khelp_deregister_helper(struct helper *h
if (!error) {
if (h->h_nhooks > 0) {
for (i = 0; i < h->h_nhooks; i++)
- khelp_remove_hhook(&h->h_hooks[i]);
+ hhook_remove_hook_lookup(&h->h_hooks[i]);
}
osd_deregister(OSD_KHELP, h->h_id);
}
@@ -263,28 +264,13 @@ khelp_get_id(char *hname)
int
khelp_add_hhook(struct hookinfo *hki, uint32_t flags)
{
- VNET_ITERATOR_DECL(vnet_iter);
int error;
- error = 0;
-
/*
- * XXXLAS: If a helper is dynamically adding a helper hook function at
- * runtime using this function, we should update the helper's h_hooks
- * struct member to include the additional hookinfo struct.
+ * XXXLAS: Should probably include the functionality to update the
+ * helper's h_hooks struct member.
*/
-
- VNET_LIST_RLOCK_NOSLEEP();
- VNET_FOREACH(vnet_iter) {
- CURVNET_SET(vnet_iter);
- error = hhook_add_hook_lookup(hki, flags);
- CURVNET_RESTORE();
-#ifdef VIMAGE
- if (error)
- break;
-#endif
- }
- VNET_LIST_RUNLOCK_NOSLEEP();
+ error = hhook_add_hook_lookup(hki, flags);
return (error);
}
@@ -292,28 +278,13 @@ khelp_add_hhook(struct hookinfo *hki, ui
int
khelp_remove_hhook(struct hookinfo *hki)
{
- VNET_ITERATOR_DECL(vnet_iter);
int error;
- error = 0;
-
/*
- * XXXLAS: If a helper is dynamically removing a helper hook function at
- * runtime using this function, we should update the helper's h_hooks
- * struct member to remove the defunct hookinfo struct.
+ * XXXLAS: Should probably include the functionality to update the
+ * helper's h_hooks struct member.
*/
-
- VNET_LIST_RLOCK_NOSLEEP();
- VNET_FOREACH(vnet_iter) {
- CURVNET_SET(vnet_iter);
- error = hhook_remove_hook_lookup(hki);
- CURVNET_RESTORE();
-#ifdef VIMAGE
- if (error)
- break;
-#endif
- }
- VNET_LIST_RUNLOCK_NOSLEEP();
+ error = hhook_remove_hook_lookup(hki);
return (error);
}
Modified: head/sys/sys/hhook.h
==============================================================================
--- head/sys/sys/hhook.h Sat Jun 15 03:55:04 2013 (r251769)
+++ head/sys/sys/hhook.h Sat Jun 15 04:03:40 2013 (r251770)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 2010 Lawrence Stewart <lstewart at freebsd.org>
+ * Copyright (c) 2010,2013 Lawrence Stewart <lstewart at freebsd.org>
* Copyright (c) 2010 The FreeBSD Foundation
* All rights reserved.
*
More information about the svn-src-all
mailing list