You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/03 13:28:17 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #957: Add nx_ variant for fs API
xiaoxiang781216 opened a new pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957
## Summary
Will be used in the furture patch to clean up errno usage
## Impact
No
## Testing
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623155212
> I will merge this now, but I have a few questions. This still feels like a work in progress. The new nx_ versions of the functions are not prototyped in header files. Is that something you plan to add later?
All put into include/nuttx/fs/fs.h
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623159639
> > ? We talked about and API to get the stack base address. Are you planning to use that?
> > I think we should simplify the system calls in that case
>
> I will look into these tomorrow. I think that both the errno and the pthread-specific data should be in TLS by default. If we have TLS that does not depend on stack alignment, then I think that TLS should be the only mechanism that is supported for errno and pthread_specific_data.
Yes, we can unify the follow three items:
1.tcb_s::pthread_data
2.tcb_s::pterrno
3.arch specific tls.h
and implement the standard compatible and modifible errno.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623128826
@xiaoxiang781216 if you are planning to use TLS, can we please make sure the all fo the existing TLS is compatible with any new TLS functionality? We talked about and API to get the stack base address. Are you planning to use that?
I think we should simplify the system calls in that case. Currently there are only two APIs using the stack information, pthread_get_stackaddr_np() and pthread_get_stacksize_np(), there are two separate system calls for each.
I think that there should be one system call that provides all of the stack information. Stack base, stack size, stack allocation. Then these (and any new TLS functions) can get the information using only a signel system call.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623178027
I just submitted PR #962 that implements the first baby step: A common API to get the stack address.
In order to be compatible with standard TLS, the data must be stored at the lower-address end of the stack (in standard TLS, the SP is AND-ed with a mask to bet the TLS location. That gets the allocation base address of the stack. A push down stack will start at the highest address and be decremented down. On stack overflow it will clobber the TLS data.
It is possible that the existence of data at the lower address will cause a problem for stack coloration and stack checking logic.
It would be preferable to create the TLS data at the "beginning" (ie., hight address) of the stack using up_stack_frame, but think that cannot be done easily for a variety of reasons.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623178027
I just submitted PR #962 that implements the first baby step: A common API to get the stack address.
In order to be compatible with standard TLS, the data must be stores at the lower end of the stack (in standard TLS, the SP is AND-ed with a mask to bet the TLS location. That gets the allocation base address of the stack. A push down stack will start at the highest address and be decremented down. On stack overflow it will clobber the TLS data.
It is possible that the existence of data at the lower address will cause a problem for stack coloration and stack checking logic.
It would be preferable to create the TLS data at the "beginning" (ie., hight address) of the stack using up_stack_frame, but think that cannot be done easily for a variety of reasons.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623156821
> ? We talked about and API to get the stack base address. Are you planning to use that?
>
> I think we should simplify the system calls in that case
I will look into these tomorrow. I think that both the errno and the pthread-specific data should be in TLS by default. If we have TLS that does not depend on stack alignment, then I think that TLS should be the only mechanism that is supported for errno and pthread_specific_data.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623178027
I just submitted PR #962 that implements the first baby step: A common API to get the stack address.
In order to be compatible with standard TLS, the data must be stores at the lower end of the stack (in standard TLS, the SP is AND-ed with a mask to bet the TLS location. That gets the allocation base address of the stack. A push down stack will start at the highest address and be decremented down. On stack overflow it will clobber the TLS data.
It is possible that the existence of data at the lower address will cause a problem for stack coloration and stack checking logic.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623563227
> I just submitted PR #962 that implements the first baby step: A common API to get the stack address.
@xiaoxiang781216 I will be taking a number of "baby steps" to get the job done. I have finished the analysis to implement TLS with an unaligned stack. That will be next PR. I did the analysis today, but I think I would be able to start coding until tomorrow. This will take a little time.
Then I will do one more PR to (1) make TLS unconditional, (2) move errno into TLS, and (3) add TLS checks into all implementations of up_createstack() and up_usestack(). The last one is the biggest effort.
I think it is best not to push too much in one PR. Each PR should be a single, self-contained step to the ultimate goal.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623154535
I will merge this now, but I have a few questions. This still feels like a work in progress. The new nx_ versions of the functions are not prototyped in header files. Is that something you plan to add later?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #957: Add nx_ variant for fs API
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #957:
URL: https://github.com/apache/incubator-nuttx/pull/957#issuecomment-623135866
> @xiaoxiang781216 if you are planning to use TLS, can we please make sure the all fo the existing TLS is compatible with any new TLS functionality? We talked about and API to get the stack base address. Are you planning to use that?
>
I am busy to clean up the kernel internal function to avoid touching errno directly. It's great if you can spend some time to refine __errno implementation on top of the new TLS.
> I think we should simplify the system calls in that case. Currently there are only two APIs using the stack information, pthread_get_stackaddr_np() and pthread_get_stacksize_np(), there are two separate system calls for each.
>
> I think that there should be one system call that provides all of the stack information. Stack base, stack size, stack allocation. Then these (and any new TLS functions) can get the information using only a signel system call.
Yes, it's better to package all value together if we want more than one. But I suppose that it is enough to implement user space TLS with the stack base address.
----------------------------------------------------------------
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:
users@infra.apache.org