You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by gn...@apache.org on 2020/08/23 14:24:05 UTC

[incubator-nuttx] 01/03: arm_createstack.c: Save tcb->adj_stack_size without tls overhead.

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

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

commit 87614e2efdf9ee46878560c8d50a55f81c6260ec
Author: Johannes Schock <jo...@nivus.com>
AuthorDate: Tue Aug 11 15:26:34 2020 +0200

    arm_createstack.c: Save tcb->adj_stack_size without tls overhead.
---
 arch/arm/src/common/arm_createstack.c | 67 +++++++++++++---------------------
 arch/arm/src/common/arm_usestack.c    | 68 +++++++++++++++++------------------
 arch/arm/src/common/arm_vfork.c       |  7 ++--
 3 files changed, 60 insertions(+), 82 deletions(-)

diff --git a/arch/arm/src/common/arm_createstack.c b/arch/arm/src/common/arm_createstack.c
index 94a5855..51cc7bd 100644
--- a/arch/arm/src/common/arm_createstack.c
+++ b/arch/arm/src/common/arm_createstack.c
@@ -102,26 +102,26 @@
 
 int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
 {
-  /* Add the size of the TLS information structure */
+  size_t alloc_size;
 
-  stack_size += sizeof(struct tls_info_s);
+  /* Add the size of the TLS information structure and align */
+
+  alloc_size = STACK_ALIGN_UP(stack_size + sizeof(struct tls_info_s));
 
 #ifdef CONFIG_TLS_ALIGNED
   /* The allocated stack size must not exceed the maximum possible for the
    * TLS feature.
    */
 
-  DEBUGASSERT(stack_size <= TLS_MAXSTACK);
-  if (stack_size >= TLS_MAXSTACK)
+  DEBUGASSERT(alloc_size <= TLS_MAXSTACK);
+  if (alloc_size > TLS_MAXSTACK)
     {
-      stack_size = TLS_MAXSTACK;
+      alloc_size = TLS_MAXSTACK;
+      stack_size = alloc_size - sizeof(struct tls_info_s);
     }
 #endif
 
-  /* Is there already a stack allocated of a different size?  Because of
-   * alignment issues, stack_size might erroneously appear to be of a
-   * different size.  Fortunately, this is not a critical operation.
-   */
+  /* Is there already a stack allocated of a different size? */
 
   if (tcb->stack_alloc_ptr && tcb->adj_stack_size != stack_size)
     {
@@ -146,7 +146,7 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
       if (ttype == TCB_FLAG_TTYPE_KERNEL)
         {
           tcb->stack_alloc_ptr =
-            (uint32_t *)kmm_memalign(TLS_STACK_ALIGN, stack_size);
+            (uint32_t *)kmm_memalign(TLS_STACK_ALIGN, alloc_size);
         }
       else
 #endif
@@ -154,7 +154,7 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
           /* Use the user-space allocator if this is a task or pthread */
 
           tcb->stack_alloc_ptr =
-            (uint32_t *)kumm_memalign(TLS_STACK_ALIGN, stack_size);
+            (uint32_t *)kumm_memalign(TLS_STACK_ALIGN, alloc_size);
         }
 
 #else /* CONFIG_TLS_ALIGNED */
@@ -163,14 +163,16 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
 
       if (ttype == TCB_FLAG_TTYPE_KERNEL)
         {
-          tcb->stack_alloc_ptr = (uint32_t *)kmm_malloc(stack_size);
+          tcb->stack_alloc_ptr =
+              (uint32_t *)kmm_memalign(CONFIG_STACK_ALIGNMENT, alloc_size);
         }
       else
 #endif
         {
           /* Use the user-space allocator if this is a task or pthread */
 
-          tcb->stack_alloc_ptr = (uint32_t *)kumm_malloc(stack_size);
+          tcb->stack_alloc_ptr =
+              (uint32_t *)kumm_memalign(CONFIG_STACK_ALIGNMENT, alloc_size);
         }
 #endif /* CONFIG_TLS_ALIGNED */
 
@@ -188,39 +190,21 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
 
   if (tcb->stack_alloc_ptr)
     {
-#if defined(CONFIG_STACK_COLORATION)
-      uintptr_t stack_base;
-#endif
-      size_t top_of_stack;
-      size_t size_of_stack;
-
       /* The ARM uses a push-down stack:  the stack grows toward lower
        * addresses in memory.  The stack pointer register, points to
        * the lowest, valid work address (the "top" of the stack).  Items
        * on the stack are referenced as positive word offsets from sp.
        */
 
-      top_of_stack = (uint32_t)tcb->stack_alloc_ptr + stack_size - 4;
-
-      /* The ARM stack must be aligned to 8-byte alignment for EABI.
-       * If necessary top_of_stack must be rounded down to the next
-       * boundary
-       */
-
-      top_of_stack = STACK_ALIGN_DOWN(top_of_stack);
-
-      /* The size of the stack in bytes is then the difference between
-       * the top and the bottom of the stack (+4 because if the top
-       * is the same as the bottom, then the size is one 32-bit element).
-       * The size need not be aligned.
+      /* Since both stack_alloc_ptr and alloc_size are in
+       * CONFIG_STACK_ALIGNMENT, and the stack ptr is decremented before
+       * the first write, we can directly save our variables to struct
+       * tcb_s.
        */
 
-      size_of_stack = top_of_stack - (uint32_t)tcb->stack_alloc_ptr + 4;
-
-      /* Save the adjusted stack values in the struct tcb_s */
-
-      tcb->adj_stack_ptr  = (uint32_t *)top_of_stack;
-      tcb->adj_stack_size = size_of_stack;
+      tcb->adj_stack_size = stack_size;
+      tcb->adj_stack_ptr  = (FAR void *)((uintptr_t)tcb->stack_alloc_ptr +
+          alloc_size);
 
       /* Initialize the TLS data structure */
 
@@ -232,11 +216,8 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
        * water marks.
        */
 
-      stack_base = (uintptr_t)tcb->stack_alloc_ptr +
-                   sizeof(struct tls_info_s);
-      stack_size = tcb->adj_stack_size -
-                   sizeof(struct tls_info_s);
-      arm_stack_color((FAR void *)stack_base, stack_size);
+      arm_stack_color((FAR void *)((uintptr_t)tcb->adj_stack_ptr -
+          tcb->adj_stack_size), tcb->adj_stack_size);
 
 #endif /* CONFIG_STACK_COLORATION */
 
diff --git a/arch/arm/src/common/arm_usestack.c b/arch/arm/src/common/arm_usestack.c
index 593cacf..9975578 100644
--- a/arch/arm/src/common/arm_usestack.c
+++ b/arch/arm/src/common/arm_usestack.c
@@ -87,9 +87,6 @@
 
 int up_use_stack(struct tcb_s *tcb, void *stack, size_t stack_size)
 {
-  size_t top_of_stack;
-  size_t size_of_stack;
-
 #ifdef CONFIG_TLS_ALIGNED
   /* Make certain that the user provided stack is properly aligned */
 
@@ -105,51 +102,54 @@ int up_use_stack(struct tcb_s *tcb, void *stack, size_t stack_size)
       up_release_stack(tcb, tcb->flags & TCB_FLAG_TTYPE_MASK);
     }
 
+  /* The ARM uses a push-down stack:  the stack grows toward lower
+   * addresses in memory.  The stack pointer register, points to
+   * the lowest, valid work address (the "top" of the stack).  Items
+   * on the stack are referenced as positive word offsets from sp.
+   */
+
+  /* We align all sizes and pointer to CONFIG_STACK_ALIGNMENT.
+   * Since the stack ptr is decremented before
+   * the first write, we can directly save our variables to struct
+   * tcb_s.
+   */
+
   /* Save the new stack allocation */
 
   tcb->stack_alloc_ptr = stack;
 
-  /* The ARM uses a push-down stack:  the stack grows toward lower addresses
-   * in memory.  The stack pointer register, points to the lowest, valid
-   * work address (the "top" of the stack).  Items on the stack are
-   * referenced as positive word offsets from sp.
-   */
+  /* Align stack top */
 
-  top_of_stack = (uint32_t)tcb->stack_alloc_ptr + stack_size - 4;
+  tcb->adj_stack_ptr =
+      (FAR void *)STACK_ALIGN_DOWN((uintptr_t)stack + stack_size);
 
-  /* The ARM stack must be aligned to 8-byte alignment for EABI.
-   * If necessary top_of_stack must be rounded down to the next
-   * boundary
-   */
+  /* Offset by tls_size */
 
-  top_of_stack = STACK_ALIGN_DOWN(top_of_stack);
+  stack = (FAR void *)((uintptr_t)stack + sizeof(struct tls_info_s));
 
-  /* The size of the stack in bytes is then the difference between
-   * the top and the bottom of the stack (+4 because if the top
-   * is the same as the bottom, then the size is one 32-bit element).
-   * The size need not be aligned.
-   */
+  /* Is there enough room for at least TLS ? */
 
-  size_of_stack = top_of_stack - (uint32_t)tcb->stack_alloc_ptr + 4;
+  if ((uintptr_t)stack <= (uintptr_t)tcb->adj_stack_ptr)
+    {
+      tcb->adj_stack_size = (uintptr_t)tcb->adj_stack_ptr - (uintptr_t)stack;
 
-  /* Save the adjusted stack values in the struct tcb_s */
+      /* Initialize the TLS data structure */
 
-  tcb->adj_stack_ptr  = (uint32_t *)top_of_stack;
-  tcb->adj_stack_size = size_of_stack;
+      memset(tcb->stack_alloc_ptr, 0, sizeof(struct tls_info_s));
 
-  /* Initialize the TLS data structure */
+    #ifdef CONFIG_STACK_COLORATION
+      /* If stack debug is enabled, then fill the stack with a
+       * recognizable value that we can use later to test for high
+       * water marks.
+       */
 
-  memset(tcb->stack_alloc_ptr, 0, sizeof(struct tls_info_s));
+      arm_stack_color((FAR void *)((uintptr_t)tcb->adj_stack_ptr -
+          tcb->adj_stack_size), tcb->adj_stack_size);
 
-#ifdef CONFIG_STACK_COLORATION
-  /* If stack debug is enabled, then fill the stack with a recognizable
-   * value that we can use later to test for high water marks.
-   */
+    #endif /* CONFIG_STACK_COLORATION */
 
-  arm_stack_color((FAR void *)((uintptr_t)tcb->stack_alloc_ptr +
-                 sizeof(struct tls_info_s)),
-                 tcb->adj_stack_size - sizeof(struct tls_info_s));
-#endif
+      return OK;
+    }
 
-  return OK;
+  return ERROR;
 }
diff --git a/arch/arm/src/common/arm_vfork.c b/arch/arm/src/common/arm_vfork.c
index 6179188..7d33d91 100644
--- a/arch/arm/src/common/arm_vfork.c
+++ b/arch/arm/src/common/arm_vfork.c
@@ -126,12 +126,9 @@ pid_t up_vfork(const struct vfork_s *context)
 
   sinfo("TCBs: Parent=%p Child=%p\n", parent, child);
 
-  /* Get the size of the parent task's stack.  Due to alignment operations,
-   * the adjusted stack size may be smaller than the stack size originally
-   * requested.
-   */
+  /* Get the size of the parent task's stack. */
 
-  stacksize = parent->adj_stack_size + CONFIG_STACK_ALIGNMENT - 1;
+  stacksize = parent->adj_stack_size;
 
   /* Allocate the stack for the TCB */