Fix #532, Set pthread names to match CFE tasks names

[dsburns]

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:

1. Set #define OS_HAVE_PTHREAD_SETNAME_NP in osconfig for target
2. Run CFE environment
3. Run htop under Linux and verify thread names appear

Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.

• API Change: Adds a task_name parameter to the OS_Posix_InternalTaskCreate_Impl function
• Behavior Change: None

System(s) tested on

• Hardware: PC
• OS: Ubuntu 18.04
• Versions: cFE 6.7.0, OSAL 5.0.0

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]

[CDKnightNASA]

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?

[dsburns]

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]

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).

[skliper]

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.

[jphickey]

What about a module/plug-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.

[astrogeco]

Context: I asked Daniel to submit the PR given that he implemented this for a mission and there is a valid usecase. @acudmore thoughts?

[jphickey]

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.

[ghost]

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.
The smallsat group needs to upgrade to the cFE 6.8 and the corresponding OSAL in the future for the bug fixes and improvements to Linux exception handling, so I would suggest the following:

• keep the patch in the SmallSat repo for now, since it provides value to the projects
• the community can work on a more modular solution that we can also use to implement the CPU affinity code that Joe mentioned
• when the smallsat group upgrades to the next cFE/OSAL hopefully we will have the solution implemented so a patch does not need to be re-applied.

[dsburns]

If you guys provide a bit of direction on the callback architecture, I'm willing to put something together in my free time.

[jphickey]

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 pthread_setname_np in the event handler for OS_EVENT_TASK_STARTUP. The handler would reside in the CFE PSP for pc-linux, and OSAL doesn't really have any part of it, other than to invoke the event notification at the right time.

[skliper]

@dsburns is the #541 approach acceptable? If so I'd like to close this issue.

[dsburns]

Looks good to me.

[astrogeco]

Looks good to me.

great, I'll close this PR then. Thanks again for getting this started!

[astrogeco]

Closing since #541 addresses the intent of this.