Fix #532, Set pthread names to match CFE tasks names#533
Fix #532, Set pthread names to match CFE tasks names#533dsburns wants to merge 1 commit intonasa:rc-5.0.0from
Conversation
This change allows underlying OS tools to view thread names for platforms that support the pthread_setname_np function.
|
A lot of other OSAL "objects" (mutex, semaphores, etc. For example, see https://github.com/nasa/osal/blob/master/src/os/shared/src/osapi-mutex.c ). These all follow the same basic pattern, and include names and can map from name to ID and back (at the OSAL layer.) Would it be worthwhile to use/support this same architecture for tasks, and reflect the names down to the OS for those that support naming but otherwise have name stored in OSAL? |
|
The OS_TaskCreate function already takes a task_name parameter. The parameter is copied to the task_name field of the OS_task_internal_record_t struct for the associated task ID. It appears this implementation already matches the mutex/sempahore implementation (unless i misinterpreted your comment). The proposed solution leverages the already existing task_name field for the OSAL task to set the pthread name in OS’s that support this function. |
jphickey
left a comment
There was a problem hiding this comment.
This needs further discussion. I'm not seeing the cost/benefit tradeoff to adding something which is non-posix. Conditional compiles are not desirable (we do them when we have to, but they are a maintenance issue, and prefer to avoid them).
|
What about a module/plug-in? Not in the core code (or even necessary as part of the framework), but user's who want it could add it in. |
I was thinking it could be useful in a generic sense to allow the BSP to register a callback function to implement additional logic on certain ops. Then the BSP can invoke the non-posix function, which would be fine. |
|
Context: I asked Daniel to submit the PR given that he implemented this for a mission and there is a valid usecase. @acudmore thoughts? |
|
I'm not saying its not "nice to have" -- it does seem useful, but it is non-standard/non-posix. I'm fine with a solution that confines the non-posix aspects to a separate file in the BSP. FWIW, we might have to do something similar for thread CPU affinity so its worthwhile to come up with a proper way to isolate this stuff not just conditional compile which is hard to maintain. |
|
This is a tough one.. I do see the usefulness of it, but I do agree with Joe, he has worked hard to get non-standard code and ifdefs out of the OSAL implementations.
|
|
If you guys provide a bit of direction on the callback architecture, I'm willing to put something together in my free time. |
|
FYI - see issue #540 and PR #541 for my suggestions. I think this general structure would work for several purposes. In relation to this ticket, it is fairly easy/trivial to call |
|
Looks good to me. |
great, I'll close this PR then. Thanks again for getting this started! |
|
Closing since #541 addresses the intent of this. |
Describe the contribution
This change allows underlying OS tools to view thread names for platforms that support the pthread_setname_np function.
Testing performed
Steps taken to test the contribution:
Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.
System(s) tested on
Additional context
None
Third party code
None
Contributor Info - All information REQUIRED for consideration of pull request
Daniel Burns
GSFC - Code 596.0
[email protected]