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