[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [UNIKRAFT PATCH] lib/ukalloc: make uk_alloc_get_default static inline



Hi Hugo,

this patch looks good. Could you rename uk_alloc_head to _uk_alloc_head? This is to show that this symbol is still internally used by the library although it is exported because of performance reasons.

Thanks,

Simon

On 08.04.20 20:24, Hugo Lefeuvre wrote:
Every allocation done through the libc malloc wrapper triggers a function
call to uk_alloc_get_default in order to get a pointer to the default
allocator.  This is not great, because the allocation path is performance
critical, and we should avoid any superflous overhead there.

This patch removes the capability to change the default allocator once it
has been set. This never really worked anyways... the libc wrapper would
forward free() calls to the new allocator, potentially resulting in memory
corruption if the new allocator doesn't handle these foreign pointers
correctly, and memory leaks because these pointers won't be freed anymore.

We make the uk_alloc_head variable global and uk_alloc_get_default static
inline.

Retrieving the default allocator becomes less expensive: a single memory
read instead of a function call + memory read.

It appears that uk_alloc_set_default was not called anywhere, so this patch
does not require any additional changes in external libraries.

Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>
---
  lib/ukalloc/alloc.c            | 39 +--------------------------------------
  lib/ukalloc/exportsyms.uk      |  2 +-
  lib/ukalloc/include/uk/alloc.h | 10 +++++++---
  3 files changed, 9 insertions(+), 42 deletions(-)

diff --git a/lib/ukalloc/alloc.c b/lib/ukalloc/alloc.c
index 214fbd9..11f58dc 100644
--- a/lib/ukalloc/alloc.c
+++ b/lib/ukalloc/alloc.c
@@ -57,7 +57,7 @@
        (ALIGN_UP((unsigned long)(size), __PAGE_SIZE) / __PAGE_SIZE)
  #define page_off(x) ((unsigned long)(x) & (__PAGE_SIZE - 1))
-static struct uk_alloc *uk_alloc_head;
+struct uk_alloc *uk_alloc_head;
int uk_alloc_register(struct uk_alloc *a)
  {
@@ -76,43 +76,6 @@ int uk_alloc_register(struct uk_alloc *a)
        return 0;
  }
-struct uk_alloc *uk_alloc_get_default(void)
-{
-       return uk_alloc_head;
-}
-
-int uk_alloc_set_default(struct uk_alloc *a)
-{
-       struct uk_alloc *head, *this, *prev;
-
-       head = uk_alloc_get_default();
-
-       if (a == head)
-               return 0;
-
-       if (!head) {
-               uk_alloc_head = a;
-               return 0;
-       }
-
-       this = head;
-       while (this->next) {
-               prev = this;
-               this = this->next;
-               if (a == this) {
-                       prev->next = this->next;
-                       this->next = head->next;
-                       head = this;
-                       return 0;
-               }
-       }
-
-       /* a is not registered yet. Add in front of the queue. */
-       a->next = head;
-       uk_alloc_head = a;
-       return 0;
-}
-
  struct metadata_ifpages {
        unsigned long   num_pages;
        void            *base;
diff --git a/lib/ukalloc/exportsyms.uk b/lib/ukalloc/exportsyms.uk
index 594999f..903565d 100644
--- a/lib/ukalloc/exportsyms.uk
+++ b/lib/ukalloc/exportsyms.uk
@@ -1,6 +1,5 @@
  uk_alloc_register
  uk_alloc_get_default
-uk_alloc_set_default
  uk_malloc_ifpages
  uk_free_ifpages
  uk_realloc_ifpages
@@ -14,3 +13,4 @@ uk_memalign_compat
  uk_realloc_compat
  uk_palloc_compat
  uk_pfree_compat
+uk_alloc_head
diff --git a/lib/ukalloc/include/uk/alloc.h b/lib/ukalloc/include/uk/alloc.h
index 73b3a45..3d1f44f 100644
--- a/lib/ukalloc/include/uk/alloc.h
+++ b/lib/ukalloc/include/uk/alloc.h
@@ -53,9 +53,6 @@ extern "C" {
  #define uk_zalloc(a, size)  uk_calloc(a, 1, size)
  #define uk_do_zalloc(a, size) uk_do_calloc(a, 1, size)
-struct uk_alloc *uk_alloc_get_default(void);
-int uk_alloc_set_default(struct uk_alloc *a);
-
  typedef void* (*uk_alloc_malloc_func_t)
                (struct uk_alloc *a, size_t size);
  typedef void* (*uk_alloc_calloc_func_t)
@@ -108,6 +105,13 @@ struct uk_alloc {
        int8_t priv[];
  };
+extern struct uk_alloc *uk_alloc_head;
+
+static inline struct uk_alloc *uk_alloc_get_default(void)
+{
+       return uk_alloc_head;
+}
+
  /* wrapper functions */
  static inline void *uk_do_malloc(struct uk_alloc *a, size_t size)
  {




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.