Fix #1281, Rtems On task creation register name.

[thesamprice]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I signed and emailed the appropriate Contributor License Agreement to GSFC-SoftwareRelease@mail.nasa.gov and copied cfs-program@lists.nasa.gov.

Describe the contribution
Registers task name with rtems on creation
Fixes #1281

Testing performed
Steps taken to test the contribution:
Ran on microblaze, typed cpuuse in rtems task, saw names.

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

• API Change: xxx (if applicable)
• Behavior Change: xxx (if applicable)
• Or no impact to behavior

System(s) tested on

• Hardware: microblaze kcu105
• OS:RTEMS

Additional context
Add any other context about the contribution here.

Third party code
If included, identify any third party code and provide text file of license

Contributor Info - All information REQUIRED for consideration of pull request
Nasa/ GSFC

[dzbaker]

CCB 25 Aug 2022: Need to test in RTEMS on our end, have someone review.

[skliper]

Related workflows: https://github.com/nasa/cFS/blob/main/.github/workflows/build-cfs-rtems4.11.yml https://github.com/nasa/cFS/blob/main/.github/workflows/build-cfs-rtems5.yml

[thesamprice]

I did this wrong,
This call should be used.
rtems_status_code rtems_object_set_name( rtems_id id, const char *name );

https://github.com/RTEMS/rtems/blob/6abdd89f191a5e6d64055093f68a4fce10554f82/cpukit/include/rtems/rtems/object.h#L437

The functions starting with _ are private rtems functions.
Also my method is missing a lock. See the pthread version.
https://github.com/RTEMS/rtems/blob/f381e9bab29278e4434b1a93e70d17a7562dc64c/cpukit/posix/src/pthreadsetnamenp.c#L40

[thesamprice]

I guess the pthread setname does the same
@DrRTEMS I think Setting the name on create requires 4 char names. where these calls allow for longer names.
The osal layer allows for longer names
DrRTEMS — Today at 12:24 PM
I would make sure the longer names work with Classic API tasks/ Inside the score, an object class can have no name, 32-bits, or strings. It probably will only take the first four characters if it works
thesamprice — Today at 12:24 PM
is there any requirement on them being unique?
right now osal gives all the tasks unique ids rather than human readable names
DrRTEMS — Today at 12:25 PM
None unless you want to use the "ident" services. They find the first match.

[travis-r-wheatley]

Replaced call to _Thread_set_name() with a call to rtems_object_set_name()

[dzbaker]

CCB 6 October 2022: Need to verify that this still works in RTEMS 4.11. @travis-r-wheatley will check which version of RTEMS he tested with.

[travis-r-wheatley]

Verified that RTEMS 6 was used for testing.

[thesamprice]

@dzbaker This function call exists in the 4.11 version of rtems.
https://github.com/RTEMS/rtems/blob/4.11/cpukit/rtems/src/rtemsobjectsetname.c
https://github.com/RTEMS/rtems/blob/4.11/cpukit/rtems/include/rtems/rtems/object.h#L155

[dzbaker]

Thank you for your contribution @thesamprice and @travis-r-wheatley, this looks great! I ran the unit test suite on RTEMS 5 and did come across a few new unit test failures:

oscore-test:
[ FAIL] 42.001 ut_oscore_task_test.c:500 - OS_TaskGetId() (0) == 65608
[ FAIL] 39.001 ut_oscore_task_test.c:379 - OS_TaskGetInfo(g_task_ids[1], &task_prop) (0) == OS_ERR_INVALID_ID (-16)
[ FAIL] 38.002 ut_oscore_task_test.c:316 - OS_TaskInstallDeleteHandler() (-16) == OS_SUCCESS
[ FAIL] 38.003 ut_oscore_task_test.c:317 - OS_TaskInstallDeleteHandler() callback invoked

Would you be able to look into these? I will be creating an RTEMS Unit Test workflow for OSAL, so that should make it easier to check in the future.

[thesamprice]

@dzbaker Did this pass on the current dev/main branch or is this branch the first to run the test suite.

[dzbaker]

@dzbaker Did this pass on the current dev/main branch or is this branch the first to run the test suite.

@thesamprice I had run this on both the current main branch and 1281-rtems-task-names. The 4 failures only showed up for me on 1281-rtems-task-names (there were other failures in other tests, but those occurred in the main branch as well). These 4 failures also didn't show up several weeks ago when I ran the unit tests on the original commit for this PR.

[thesamprice]

@dzbaker
I think the issue is how tasks are gotten.
https://github.com/nasa/osal/blob/main/src/os/rtems/src/os-impl-tasks.c#L303

I think this should be scanning through and using impl->id rather than task names.
impl->id is set at startup, and then were changing the name.
status = rtems_task_create(r_name, task->priority, task->stack_size, r_mode, r_attributes, &impl->id);
Is there a cfs reverse map call for this type of logic?

[dzbaker]

@dzbaker I think the issue is how tasks are gotten. https://github.com/nasa/osal/blob/main/src/os/rtems/src/os-impl-tasks.c#L303

I think this should be scanning through and using impl->id rather than task names. impl->id is set at startup, and then were changing the name. status = rtems_task_create(r_name, task->priority, task->stack_size, r_mode, r_attributes, &impl->id); Is there a cfs reverse map call for this type of logic?

I'm not quite sure. @skliper @dmknutsen, would you know?

[skliper]

Just took a quick skim... and yeah, if you are changing the name this get task id implementation won't work. Maybe a pattern like the Linux or VxWorks implementation makes more sense? I'd start there for ideas.

[jphickey]

Confirm that the classic RTEMS task name is already used for the purpose of keeping the OSAL ID - this cannot be changed without breaking stuff. On the original review I thought there was an alternate name that this was setting.

Note that classic RTEMS task "names" here are only 4 characters long, so they really aren't that useful for human/user friendly consumption. They are too short for anything like that. But they are big enough to be useful for stashing the OSAL ID.

The whole point of stashing the OSAL ID this way was to avoid searching through the table. As a result the current implementation of OS_TaskGetId() can be fast and lockless - by using some form of thread local storage we avoid needing to search the table for a match.

• VxWorks leverages the fact that we can specify the TCB structure - which will be a pointer offset from the OSAL record.
• POSIX has a "getspecific" call for arbitrary thread local variables
• RTEMS classic API didn't seem to have anything exactly equivalent to either of those, but because the "name" can hold any arbitrary data, and its not useful as an actual descriptive name, it worked.

If this current name lookup is replaced with a search, then the whole thing needs to be done under lock, and that will have a serious increase on the time cost of this call.

[chillfig]

CCB:2023-03-09: Will not merge.