You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2021/04/20 03:20:27 UTC

[incubator-nuttx] branch master updated: modlib: Always use separate allocation for text and data

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 418e11b  modlib: Always use separate allocation for text and data
418e11b is described below

commit 418e11b8b38855eacf55deea8d658c5760b51488
Author: YAMAMOTO Takashi <ya...@midokura.com>
AuthorDate: Thu Apr 15 11:33:48 2021 +0900

    modlib: Always use separate allocation for text and data
    
    Pros:
    
    * Reduce code differences
    * Smaller allocations for !CONFIG_ARCH_USE_MODULE_TEXT
    
    Cons:
    
    * Likely to use more memory for !CONFIG_ARCH_USE_MODULE_TEXT in total
    
    Tested with:
    
    * sim:module on macOS
    * esp32-devkit:nsh + CONFIG_MODULE on qemu
    * lm3s6965-ek:qemu-protected + CONFIG_EXAMPLES_SOTEST on qemu
---
 include/nuttx/lib/modlib.h       |  4 ----
 libs/libc/dlfcn/lib_dlclose.c    | 19 ++++++++++++++++---
 libs/libc/dlfcn/lib_dlopen.c     |  3 ++-
 libs/libc/modlib/modlib_load.c   | 35 +++++------------------------------
 libs/libc/modlib/modlib_unload.c | 11 ++++-------
 sched/module/mod_insmod.c        |  4 ----
 sched/module/mod_procfs.c        |  8 --------
 sched/module/mod_rmmod.c         | 11 +++--------
 8 files changed, 30 insertions(+), 65 deletions(-)

diff --git a/include/nuttx/lib/modlib.h b/include/nuttx/lib/modlib.h
index bd2b363..6b72429 100644
--- a/include/nuttx/lib/modlib.h
+++ b/include/nuttx/lib/modlib.h
@@ -159,12 +159,8 @@ struct module_s
   mod_initializer_t initializer;       /* Module initializer function */
 #endif
   struct mod_info_s modinfo;           /* Module information */
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
   FAR void *textalloc;                 /* Allocated kernel text memory */
   FAR void *dataalloc;                 /* Allocated kernel memory */
-#else
-  FAR void *alloc;                     /* Allocated kernel memory */
-#endif
 #if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
   size_t textsize;                     /* Size of the kernel .text memory allocation */
   size_t datasize;                     /* Size of the kernel .bss/.data memory allocation */
diff --git a/libs/libc/dlfcn/lib_dlclose.c b/libs/libc/dlfcn/lib_dlclose.c
index 24118e5..414c32b 100644
--- a/libs/libc/dlfcn/lib_dlclose.c
+++ b/libs/libc/dlfcn/lib_dlclose.c
@@ -112,17 +112,30 @@ static inline int dlremove(FAR void *handle)
 
   /* Release resources held by the module */
 
-  if (modp->alloc != NULL)
+  if (modp->textalloc != NULL)
     {
       /* Free the module memory */
 
-      lib_free((FAR void *)modp->alloc);
+      lib_free((FAR void *)modp->textalloc);
 
       /* Nullify so that the memory cannot be freed again */
 
-      modp->alloc = NULL;
+      modp->textalloc = NULL;
 #if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
       modp->textsize  = 0;
+#endif
+    }
+
+  if (modp->dataalloc != NULL)
+    {
+      /* Free the module memory */
+
+      lib_free((FAR void *)modp->dataalloc);
+
+      /* Nullify so that the memory cannot be freed again */
+
+      modp->dataalloc = NULL;
+#if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
       modp->datasize  = 0;
 #endif
     }
diff --git a/libs/libc/dlfcn/lib_dlopen.c b/libs/libc/dlfcn/lib_dlopen.c
index 983434e..cdeaf29 100644
--- a/libs/libc/dlfcn/lib_dlopen.c
+++ b/libs/libc/dlfcn/lib_dlopen.c
@@ -208,7 +208,8 @@ static inline FAR void *dlinsert(FAR const char *filename)
 
   /* Save the load information */
 
-  modp->alloc       = (FAR void *)loadinfo.textalloc;
+  modp->textalloc       = (FAR void *)loadinfo.textalloc;
+  modp->dataalloc       = (FAR void *)loadinfo.datastart;
 #if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
   modp->textsize    = loadinfo.textsize;
   modp->datasize    = loadinfo.datasize;
diff --git a/libs/libc/modlib/modlib_load.c b/libs/libc/modlib/modlib_load.c
index 839e9d0..efe936d 100644
--- a/libs/libc/modlib/modlib_load.c
+++ b/libs/libc/modlib/modlib_load.c
@@ -241,10 +241,6 @@ static inline int modlib_loadfile(FAR struct mod_loadinfo_s *loadinfo)
 
 int modlib_load(FAR struct mod_loadinfo_s *loadinfo)
 {
-#if !defined(CONFIG_ARCH_USE_MODULE_TEXT)
-  size_t align;
-  size_t text_size;
-#endif
   int ret;
 
   binfo("loadinfo: %p\n", loadinfo);
@@ -267,12 +263,16 @@ int modlib_load(FAR struct mod_loadinfo_s *loadinfo)
 
   /* Allocate memory to hold the ELF image */
 
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
   if (loadinfo->textsize > 0)
     {
+#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
       loadinfo->textalloc = (uintptr_t)
                             up_module_text_memalign(loadinfo->textalign,
                                                     loadinfo->textsize);
+#else
+      loadinfo->textalloc = (uintptr_t)lib_memalign(loadinfo->textalign,
+                                                    loadinfo->textsize);
+#endif
       if (!loadinfo->textalloc)
         {
           berr("ERROR: Failed to allocate memory for the module text\n");
@@ -292,31 +292,6 @@ int modlib_load(FAR struct mod_loadinfo_s *loadinfo)
           goto errout_with_buffers;
         }
     }
-#else
-  align = loadinfo->textalign;
-  if (align < loadinfo->dataalign)
-    {
-      align = loadinfo->dataalign;
-    }
-
-  text_size = loadinfo->textsize;
-  if (loadinfo->datasize > 0)
-    {
-      text_size = _ALIGN_UP(text_size, loadinfo->dataalign);
-    }
-
-  loadinfo->textalloc = (uintptr_t)lib_memalign(align,
-                                                text_size +
-                                                loadinfo->datasize);
-  if (!loadinfo->textalloc)
-    {
-      berr("ERROR: Failed to allocate memory for the module\n");
-      ret = -ENOMEM;
-      goto errout_with_buffers;
-    }
-
-  loadinfo->datastart = loadinfo->textalloc + text_size;
-#endif
 
   /* Load ELF section data into memory */
 
diff --git a/libs/libc/modlib/modlib_unload.c b/libs/libc/modlib/modlib_unload.c
index fd83fa8..0e4a61a 100644
--- a/libs/libc/modlib/modlib_unload.c
+++ b/libs/libc/modlib/modlib_unload.c
@@ -58,22 +58,19 @@ int modlib_unload(struct mod_loadinfo_s *loadinfo)
 
   /* Release memory holding the relocated ELF image */
 
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
   if (loadinfo->textalloc != 0)
     {
+#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
       up_module_text_free((FAR void *)loadinfo->textalloc);
+#else
+      lib_free((FAR void *)loadinfo->textalloc);
+#endif
     }
 
   if (loadinfo->datastart != 0)
     {
       lib_free((FAR void *)loadinfo->datastart);
     }
-#else
-  if (loadinfo->textalloc != 0)
-    {
-      lib_free((FAR void *)loadinfo->textalloc);
-    }
-#endif
 
   /* Clear out all indications of the allocated address environment */
 
diff --git a/sched/module/mod_insmod.c b/sched/module/mod_insmod.c
index a90c455..438e7a0 100644
--- a/sched/module/mod_insmod.c
+++ b/sched/module/mod_insmod.c
@@ -224,12 +224,8 @@ FAR void *insmod(FAR const char *filename, FAR const char *modname)
 
   /* Save the load information */
 
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
   modp->textalloc   = (FAR void *)loadinfo.textalloc;
   modp->dataalloc   = (FAR void *)loadinfo.datastart;
-#else
-  modp->alloc       = (FAR void *)loadinfo.textalloc;
-#endif
 #if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
   modp->textsize    = loadinfo.textsize;
   modp->datasize    = loadinfo.datasize;
diff --git a/sched/module/mod_procfs.c b/sched/module/mod_procfs.c
index e144d3f..1a8b50f 100644
--- a/sched/module/mod_procfs.c
+++ b/sched/module/mod_procfs.c
@@ -138,17 +138,9 @@ static int modprocfs_callback(FAR struct module_s *modp, FAR void *arg)
                       modp->modname, modp->initializer,
                       modp->modinfo.uninitializer, modp->modinfo.arg,
                       modp->modinfo.nexports,
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
                       modp->textalloc,
-#else
-                      modp->alloc,
-#endif
                       (unsigned long)modp->textsize,
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
                       (FAR uint8_t *)modp->dataalloc,
-#else
-                      (FAR uint8_t *)modp->alloc + modp->textsize,
-#endif
                       (unsigned long)modp->datasize);
   copysize = procfs_memcpy(priv->line, linesize, priv->buffer,
                            priv->remaining, &priv->offset);
diff --git a/sched/module/mod_rmmod.c b/sched/module/mod_rmmod.c
index 82ca9ac..61101f8 100644
--- a/sched/module/mod_rmmod.c
+++ b/sched/module/mod_rmmod.c
@@ -113,11 +113,7 @@ int rmmod(FAR void *handle)
 
   /* Release resources held by the module */
 
-#if defined(CONFIG_ARCH_USE_MODULE_TEXT)
   if (modp->textalloc != NULL || modp->dataalloc != NULL)
-#else
-  if (modp->alloc != NULL)
-#endif
     {
       /* Free the module memory
        * and nullify so that the memory cannot be freed again
@@ -125,13 +121,12 @@ int rmmod(FAR void *handle)
 
 #if defined(CONFIG_ARCH_USE_MODULE_TEXT)
       up_module_text_free((FAR void *)modp->textalloc);
+#else
+      kmm_free((FAR void *)modp->textalloc);
+#endif
       kmm_free((FAR void *)modp->dataalloc);
       modp->textalloc = NULL;
       modp->dataalloc = NULL;
-#else
-      kmm_free((FAR void *)modp->alloc);
-      modp->alloc = NULL;
-#endif
 #if defined(CONFIG_FS_PROCFS) && !defined(CONFIG_FS_PROCFS_EXCLUDE_MODULE)
       modp->textsize  = 0;
       modp->datasize  = 0;