From owner-svn-src-stable@FreeBSD.ORG Wed Jul 3 05:29:22 2013 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 95A018C0; Wed, 3 Jul 2013 05:29:22 +0000 (UTC) (envelope-from lstewart@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 78A071DD6; Wed, 3 Jul 2013 05:29:22 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r635TMH1035709; Wed, 3 Jul 2013 05:29:22 GMT (envelope-from lstewart@svn.freebsd.org) Received: (from lstewart@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r635TLje035704; Wed, 3 Jul 2013 05:29:21 GMT (envelope-from lstewart@svn.freebsd.org) Message-Id: <201307030529.r635TLje035704@svn.freebsd.org> From: Lawrence Stewart Date: Wed, 3 Jul 2013 05:29:21 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r252537 - in stable/8/sys: kern sys X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Jul 2013 05:29:22 -0000 Author: lstewart Date: Wed Jul 3 05:29:21 2013 New Revision: 252537 URL: http://svnweb.freebsd.org/changeset/base/252537 Log: MFC r251770: 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 Modified: stable/8/sys/kern/kern_hhook.c stable/8/sys/kern/kern_khelp.c stable/8/sys/sys/hhook.h Directory Properties: stable/8/sys/ (props changed) stable/8/sys/kern/ (props changed) stable/8/sys/sys/ (props changed) Modified: stable/8/sys/kern/kern_hhook.c ============================================================================== --- stable/8/sys/kern/kern_hhook.c Wed Jul 3 05:28:22 2013 (r252536) +++ stable/8/sys/kern/kern_hhook.c Wed Jul 3 05:29:21 2013 (r252537) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2010 Lawrence Stewart + * Copyright (c) 2010,2013 Lawrence Stewart * 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: stable/8/sys/kern/kern_khelp.c ============================================================================== --- stable/8/sys/kern/kern_khelp.c Wed Jul 3 05:28:22 2013 (r252536) +++ stable/8/sys/kern/kern_khelp.c Wed Jul 3 05:29:21 2013 (r252537) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2010 Lawrence Stewart + * Copyright (c) 2010,2013 Lawrence Stewart * 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: stable/8/sys/sys/hhook.h ============================================================================== --- stable/8/sys/sys/hhook.h Wed Jul 3 05:28:22 2013 (r252536) +++ stable/8/sys/sys/hhook.h Wed Jul 3 05:29:21 2013 (r252537) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2010 Lawrence Stewart + * Copyright (c) 2010,2013 Lawrence Stewart * Copyright (c) 2010 The FreeBSD Foundation * All rights reserved. *