<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/7600">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">loader: Rework of load_dynamic_module.<br><br>* Split off load_dlopen to perform actual dlopen, check results and log<br>  warnings when needed.<br>* Use flags which minimize number of calls to dlopen required.  First<br>  attempt always uses RTLD_GLOBAL when global_symbols_only is enabled,<br>  RTLD_LOCAL when it is not.<br><br>This patch significantly reduces the number of dlopen's performed.  With<br>299 modules my system ran dlopen 857 times before this patch, 655 times<br>after this patch.<br><br>Change-Id: Ib2c9903cfddcc01aed3e01c1e7fe4a3fb9af0f8b<br>---<br>M main/loader.c<br>1 file changed, 63 insertions(+), 55 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/00/7600/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/loader.c b/main/loader.c<br>index f06d637..f204a86 100644<br>--- a/main/loader.c<br>+++ b/main/loader.c<br>@@ -520,83 +520,91 @@<br> <br> #define MODULE_LOCAL_ONLY (void *)-1<br> <br>-static struct ast_module *load_dynamic_module(const char *resource_in, unsigned int global_symbols_only, unsigned int suppress_logging, struct ast_heap *resource_heap)<br>+/*!<br>+ * \internal<br>+ * \brief Attempt to dlopen a module.<br>+ *<br>+ * \param resource_in The module name to load.<br>+ * \param so_ext ".so" or blank if ".so" is already part of resource_in.<br>+ * \param filename Passed directly to dlopen.<br>+ * \param flags Passed directly to dlopen.<br>+ * \param suppress_logging Do not log any error from dlopen.<br>+ *<br>+ * \return Pointer to opened module, NULL on error.<br>+ *<br>+ * \warning module_list must be locked before calling this function.<br>+ */<br>+static struct ast_module *load_dlopen(const char *resource_in, const char *so_ext,<br>+     const char *filename, int flags, unsigned int suppress_logging)<br> {<br>-  char fn[PATH_MAX] = "";<br>-    void *lib = NULL;<br>     struct ast_module *mod;<br>-      unsigned int wants_global;<br>-   int space;      /* room needed for the descriptor */<br>- int missing_so = 0;<br> <br>        ast_assert(!resource_being_loaded);<br> <br>-       space = sizeof(*resource_being_loaded) + strlen(resource_in) + 1;<br>-    if (strcasecmp(resource_in + strlen(resource_in) - 3, ".so")) {<br>-            missing_so = 1;<br>-              space += 3;     /* room for the extra ".so" */<br>+     mod = ast_calloc(1, sizeof(*mod) + strlen(resource_in) + strlen(so_ext) + 1);<br>+        if (!mod) {<br>+          return NULL;<br>  }<br> <br>- snprintf(fn, sizeof(fn), "%s/%s%s", ast_config_AST_MODULE_DIR, resource_in, missing_so ? ".so" : "");<br>+  sprintf(mod->resource, "%s%s", resource_in, so_ext); /* safe */<br> <br>-      /* make a first load of the module in 'quiet' mode... don't try to resolve<br>-          any symbols, and don't export any symbols. this will allow us to peek into<br>-       the module's info block (if available) to see what flags it has set */<br>-<br>-     mod = resource_being_loaded = ast_calloc(1, space);<br>-  if (!resource_being_loaded)<br>-          return NULL;<br>- strcpy(resource_being_loaded->resource, resource_in);<br>-     if (missing_so)<br>-              strcat(resource_being_loaded->resource, ".so");<br>-<br>-      lib = dlopen(fn, RTLD_LAZY | RTLD_GLOBAL);<br>+   resource_being_loaded = mod;<br>+ mod->lib = dlopen(filename, flags);<br>        if (resource_being_loaded) {<br>          resource_being_loaded = NULL;<br>-                if (lib) {<br>+           if (mod->lib) {<br>                    ast_log(LOG_ERROR, "Module '%s' did not register itself during load\n", resource_in);<br>-                      logged_dlclose(resource_in, lib);<br>+                    logged_dlclose(resource_in, mod->lib);<br>             } else if (!suppress_logging) {<br>                       ast_log(LOG_WARNING, "Error loading module '%s': %s\n", resource_in, dlerror());<br>            }<br>             ast_free(mod);<br>+<br>             return NULL;<br>  }<br> <br>- wants_global = ast_test_flag(mod->info, AST_MODFLAG_GLOBAL_SYMBOLS);<br>+      return mod;<br>+}<br> <br>-   /* if we are being asked only to load modules that provide global symbols,<br>-      and this one does not, then close it and return */<br>-        if (global_symbols_only && !wants_global) {<br>-          logged_dlclose(resource_in, lib);<br>+static struct ast_module *load_dynamic_module(const char *resource_in, unsigned int global_symbols_only, unsigned int suppress_logging, struct ast_heap *resource_heap)<br>+{<br>+      char fn[PATH_MAX];<br>+   struct ast_module *mod;<br>+      size_t resource_in_len = strlen(resource_in);<br>+        int exports_globals;<br>+ const char *so_ext = "";<br>+<br>+        if (resource_in_len < 4 || strcasecmp(resource_in + resource_in_len - 3, ".so")) {<br>+              so_ext = ".so";<br>+    }<br>+<br>+ snprintf(fn, sizeof(fn), "%s/%s%s", ast_config_AST_MODULE_DIR, resource_in, so_ext);<br>+<br>+    /* Try loading in quiet mode first with flags to export global symbols.<br>+       * If the module does not want to export globals we will close and reopen. */<br>+        mod = load_dlopen(resource_in, so_ext, fn,<br>+           global_symbols_only ? RTLD_LAZY | RTLD_GLOBAL : RTLD_NOW | RTLD_LOCAL,<br>+               suppress_logging);<br>+<br>+        if (!mod) {<br>+          return NULL;<br>+ }<br>+<br>+ exports_globals = ast_test_flag(mod->info, AST_MODFLAG_GLOBAL_SYMBOLS);<br>+   if ((global_symbols_only && exports_globals) || (!global_symbols_only && !exports_globals)) {<br>+                /* The first dlopen had the correct flags. */<br>+                return mod;<br>+  }<br>+<br>+ /* Close the module so we can reopen with correct flags. */<br>+  logged_dlclose(resource_in, mod->lib);<br>+    if (global_symbols_only) {<br>            return MODULE_LOCAL_ONLY;<br>     }<br> <br>- logged_dlclose(resource_in, lib);<br>-<br>- /* start the load process again */<br>-   mod = resource_being_loaded = ast_calloc(1, space);<br>-  if (!resource_being_loaded)<br>-          return NULL;<br>- strcpy(resource_being_loaded->resource, resource_in);<br>-     if (missing_so)<br>-              strcat(resource_being_loaded->resource, ".so");<br>-<br>-      lib = dlopen(fn, wants_global ? RTLD_LAZY | RTLD_GLOBAL : RTLD_NOW | RTLD_LOCAL);<br>-    resource_being_loaded = NULL;<br>-        if (!lib) {<br>-          ast_log(LOG_WARNING, "Error loading module '%s': %s\n", resource_in, dlerror());<br>-           ast_free(mod);<br>-               return NULL;<br>- }<br>-<br>- /* since the module was successfully opened, and it registered itself<br>-           the previous time we did that, we're going to assume it worked this<br>-      time too :) */<br>-<br>- AST_DLLIST_LAST(&module_list)->lib = lib;<br>-<br>-  return AST_DLLIST_LAST(&module_list);<br>+    return load_dlopen(resource_in, so_ext, fn,<br>+          exports_globals ? RTLD_LAZY | RTLD_GLOBAL : RTLD_NOW | RTLD_LOCAL,<br>+           0);<br> }<br> <br> int modules_shutdown(void)<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7600">change 7600</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7600"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ib2c9903cfddcc01aed3e01c1e7fe4a3fb9af0f8b </div>
<div style="display:none"> Gerrit-Change-Number: 7600 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>