From owner-p4-projects@FreeBSD.ORG Thu May 25 22:35:00 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 1529716ADD4; Thu, 25 May 2006 22:32:05 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 882FB16AB25 for ; Thu, 25 May 2006 22:28:53 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4E1DE43D55 for ; Thu, 25 May 2006 22:28:53 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k4PMRudt078615 for ; Thu, 25 May 2006 22:27:56 GMT (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k4PMRtfK078612 for perforce@freebsd.org; Thu, 25 May 2006 22:27:55 GMT (envelope-from jhb@freebsd.org) Date: Thu, 25 May 2006 22:27:55 GMT Message-Id: <200605252227.k4PMRtfK078612@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin To: Perforce Change Reviews Cc: Subject: PERFORCE change 97831 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 May 2006 22:35:05 -0000 http://perforce.freebsd.org/chv.cgi?CH=97831 Change 97831 by jhb@jhb_mutex on 2006/05/25 22:27:03 Provide linker_release_module() as a compliment to linker_reference_module() instead of code matching calls to linker_reference_module() with calls to linker_file_unload(). Also, while I'm here, make the task to unload firmware modules less racy by claering the FP entry first and then going any unloading the module using a cached copy of the linker_file_t. Affected files ... .. //depot/projects/smpng/sys/dev/digi/digi.c#32 edit .. //depot/projects/smpng/sys/kern/kern_linker.c#60 edit .. //depot/projects/smpng/sys/kern/subr_firmware.c#3 edit .. //depot/projects/smpng/sys/sys/linker.h#22 edit Differences ... ==== //depot/projects/smpng/sys/dev/digi/digi.c#32 (text+ko) ==== @@ -795,7 +795,7 @@ free(sym, M_TEMP); if (symptr == NULL) { printf("digi_%s.ko: Symbol `%s' not found\n", sc->module, sym); - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_release_module(NULL, NULL, lf); return (EINVAL); } @@ -803,7 +803,7 @@ if (digi_mod->dm_version != DIGI_MOD_VERSION) { printf("digi_%s.ko: Invalid version %d (need %d)\n", sc->module, digi_mod->dm_version, DIGI_MOD_VERSION); - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_release_module(NULL, NULL, lf); return (EINVAL); } @@ -825,7 +825,7 @@ bcopy(digi_mod->dm_link.data, sc->link.data, sc->link.size); } - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_release_module(NULL, NULL, lf); return (0); } ==== //depot/projects/smpng/sys/kern/kern_linker.c#60 (text+ko) ==== @@ -425,7 +425,7 @@ linker_file_t *result) { modlist_t mod; - int error, locked; + int error; KLD_LOCK(); if ((mod = modlist_lookup2(modname, verinfo)) != NULL) { @@ -441,6 +441,31 @@ return (error); } +int +linker_release_module(const char *modname, struct mod_depend *verinfo, + linker_file_t lf) +{ + modlist_t mod; + int error; + + KLD_LOCK(); + if (lf == NULL) { + KASSERT(modname != NULL, + ("linker_release_module: no file or name")); + mod = modlist_lookup2(modname, verinfo); + if (mod == NULL) { + KLD_UNLOCK(); + return (ESRCH); + } + lf = mod->container; + } else + KASSERT(modname == NULL && verinfo == NULL, + ("linker_release_module: both file and name")); + error = linker_file_unload_internal(lf, LINKER_UNLOAD_NORMAL); + KLD_UNLOCK(); + return (error); +} + static linker_file_t linker_find_file_by_name(const char *filename) { ==== //depot/projects/smpng/sys/kern/subr_firmware.c#3 (text+ko) ==== @@ -206,20 +206,17 @@ unloadentry(void *unused1, int unused2) { struct firmware *fp; + linker_file_t lf; mtx_lock(&firmware_mtx); while ((fp = lookup(name_unload))) { - /* - * XXX: ugly, we should be able to lookup unlocked here if - * we properly lock around clearentry below to avoid double - * unload. Play it safe for now. - */ + lf = fp->file; + clearentry(fp, 0); mtx_unlock(&firmware_mtx); - linker_file_unload(fp->file, LINKER_UNLOAD_NORMAL); + linker_release_module(NULL, NULL, lf); mtx_lock(&firmware_mtx); - clearentry(fp, 0); } mtx_unlock(&firmware_mtx); } ==== //depot/projects/smpng/sys/sys/linker.h#22 (text+ko) ==== @@ -111,6 +111,14 @@ linker_file_t* _result); /* + * Release a reference to a module, unloading it if there are no more + * references. Note that one should either provide a module name and + * optional version info or a linker file, but not both. + */ +int linker_release_module(const char *_modname, struct mod_depend *_verinfo, + linker_file_t _file); + +/* * Unload a file, freeing up memory. */ int linker_file_unload(linker_file_t _file, int flags);