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
This commit is contained in:
Lawrence Stewart 2013-06-15 04:03:40 +00:00
parent b059b01e74
commit b1f53277ec
3 changed files with 86 additions and 58 deletions

View File

@ -1,5 +1,5 @@
/*-
* Copyright (c) 2010 Lawrence Stewart <lstewart@freebsd.org>
* Copyright (c) 2010,2013 Lawrence Stewart <lstewart@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, struct hookinfo *hki, uint32_t flags)
}
/*
* 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, struct hookinfo *hki)
}
/*
* 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, int32_t hhook_id, struct hhook_head **hh
#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 *hhh)
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 *hhh)
HHH_WUNLOCK(hhh);
HHH_LOCK_DESTROY(hhh);
free(hhh, M_HHOOK);
n_hhookheads--;
}
/*

View File

@ -1,5 +1,5 @@
/*-
* Copyright (c) 2010 Lawrence Stewart <lstewart@freebsd.org>
* Copyright (c) 2010,2013 Lawrence Stewart <lstewart@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, uint32_t flags)
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);
}

View File

@ -1,5 +1,5 @@
/*-
* Copyright (c) 2010 Lawrence Stewart <lstewart@freebsd.org>
* Copyright (c) 2010,2013 Lawrence Stewart <lstewart@freebsd.org>
* Copyright (c) 2010 The FreeBSD Foundation
* All rights reserved.
*