patacongo commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r611210648



##########
File path: sched/sched/sched_get_stackinfo.c
##########
@@ -101,5 +111,7 @@ int nxsched_get_stackinfo(pid_t pid, FAR struct stackinfo_s 
*stackinfo)
   stackinfo->adj_stack_size  = qtcb->adj_stack_size;
   stackinfo->stack_alloc_ptr = qtcb->stack_alloc_ptr;
   stackinfo->adj_stack_ptr   = qtcb->adj_stack_ptr;
+  stackinfo->tls_info_ptr    = qtcb->tls_info_ptr;

Review comment:
       > I save this poiner to fix the wrong assumption(push-down stack) you 
made in #987:
   > 
[bda24f0#diff-dd1838d4b8adff099b4fbcd041c8ec8e7f70cdf1fb9e360781b7298352105be6R68-R72](https://github.com/apache/incubator-nuttx/commit/bda24f09c2fe0d72b3452686052d075fa7f416ae#diff-dd1838d4b8adff099b4fbcd041c8ec8e7f70cdf1fb9e360781b7298352105be6R68-R72)
   
   This is not a wrong assumption and has nothing to do with a push-down stack. 
 It is correct logic.  Your change is wrong; the referenced logic is correct.  
The TLS data must lie at the lowest address of the memory allocation or it 
cannot be accessed in user space use via:
   
   tlsinfo = (FAR struct tls_info_s *)((builtin_stack_pointer()) & TLS_MASK).
   
   On a push-up stack, TLS data must still lie at the lowest allocated address. 
 The only difference is that the memory is reserved and "end" of the stack in 
one case and at "beginning" in the other.  That difference would have to be 
handled by architecture-specific code if we ever did support an architecture 
with a push-up stack.
   
           Push Down         Push Up
       +-------------+   +-------------+ <- Allocation address (lowest)
       |  TLS Data   |   |  TLS Data   |
       +-------------+   +-------------+
       |             |   |  Task Data  |
       |  Available  |   +-------------+
       |    Stack    |   |  Arguments  |
       |             |   +-------------+
       |             |   |             |
       |             |   |  Available  | |
       |             |   |    Stack    | v
       |             | ^ |             |
       +-------------+ | |             |
       |  Arguments  |   |             |
       +-------------+   |             |
       |  Task Data  |   |             |
       +-------------+   +-------------+
   
   NOTE:  The comment at the referenced location is wrong.  That code has 
nothing to do with push-up or push-down.  The code is correct.  PR #3519 fixes 
that bad comment.
   
   This kind of access (without system calls) is required for efficient, 
user-space access using aligned stacks.
   
   That is necessary for pure user-space access.  You changes are not good and 
should not be merged (although it is difficult to understand what all is going 
on in 207 files of mixed changes).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to