search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

Use of RTX layer in `mbed_error` #9125

Closed embeddedteam103 closed 3 years ago

embeddedteam103 commented 5 years ago

Description

As I mentioned in https://github.com/ARM-software/CMSIS_5/issues/479 we work with mbed-os with a different implementation of the cmsis-os abstraction layer based on the closed source threadx rtos. There are cases where internal details of rtx are used in mbed.

One of the newer ones is with the introduction of mbed_error in one of the recent releases. It uses information of the current thread: https://github.com/ARMmbed/mbed-os/blob/463a4536e24135d19346093da5e39d84d6c96db9/platform/mbed_error.c#L150

Note that AFAICT this information cannot be retrieved from the cmsis abstraction layer and the only solution for this bug is an extension of the cmsis api. Do you think such an extension is viable?

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug
0xc0170 commented 5 years ago

@SenRamakri please review. Was this usage reported back to CMSIS_5 as a issue/pull request for adding this functionality ?

I do not see this functionality covered in cmsis rtos v2.

kjbracey commented 5 years ago

As of a very recent patch release of CMSIS-RTOS 2 / RTX, osThreadGetId is now documented callable from interrupt.

I know this, because it came up in https://github.com/ARMmbed/mbed-os/pull/8961

Documentation doesn't specify what it does from exception, but it seems that RTX's implementation does return the OS's current thread, even when called from exception context. (At some point I thought it returned 0). So osThreadGetId() here might do.

The remaining points are:

The latter is never specified for any CMSIS-RTOS call, but generally it seems that if a call is documented as callable from interrupt, thread context with ISRs disabled also works, at least for RTX.

If you haven't hit it already, you will soon hit the osKernelSuspend in mbed_rtx_idle.cpp. I can't see how to do that without a race with only CMSIS-RTOS API. (See #6534 for discussion - don't think an issue was raised anywhere).

0xc0170 commented 5 years ago

@JonatanAntoni please review

embeddedteam103 commented 5 years ago

If you haven't hit it already, you will soon hit the osKernelSuspend in mbed_rtx_idle.cpp. I can't see how to do that without a race with only CMSIS-RTOS API. (See #6534 for discussion - don't think an issue was raised anywhere).

The idle thread is actually something we haven't implemented in our cmsis wrapper at all (due to differing functionality of suspend/resume in threadx) and we simply rolled our own thread_idle implementation.

We are actually planning to move to the standard one in some point. I will look at this issue to see how it would work in threadx when we finally decide to prioritze this.

JonatanAntoni commented 5 years ago

Hi @kjbracey-arm,

The remaining points are: can CMSIS-RTOS document that it returns current thread, so is good for this purpose (or profiling)? can it be called from thread context with ISRs disabled, which might happen here?

The current documentation of osThreadGetId() states

[..] returns the thread object ID of the currently running thread.

So, yes, for RTX it simply returns osRtxInfo.thread.run.curr, which is the currently scheduled thread.

And yes, it can be called from ISR context since a while (or with interrupts disabled/masked). Please be aware of possible "race conditions" when relying on this information from ISR context.

Low-power (or tickless) operation is tricky and might be implemented very differently. Hence we never tried to abstract this functionality using CMSIS-RTOS2 APIs. The API initial intention of this API is not to fully decouple user applications from the actual RTOS implementation but allow generic middle-ware components (like com stacks for instance).

Cheers, Jonatan

kjbracey commented 5 years ago

returns the thread object ID of the currently running thread.

To me that is not 100% concrete - if called from exception context, arguably that thread isn't currently "running" - it's been preempted by the exception - so I can imagine another CMSIS-RTOS implementer doing something different. Another possible definition would be to return NULL if not called from thread context, and that would make more sense if, say, using it to implement std::this_thread::get_id(); if you were permitting that to be called from actual exception context (as opposed to just ISRs disabled), it wouldn't make much sense given its name to return the current foreground thread.

(Been spending far too much time pinning down some of this "current thread" stuff in RTOS awareness in pyOCD recently.)

And yes, it can be called from ISR context since a while (or with interrupts disabled/masked).

In the past I do recall there were calls that were callable from ISR, but faulted if called from thread context with ISRs disabled. I know in RTX 4 either semaphore-post or flag-set blew up. Is this a general rule now that "callable from ISR" context now permits that case? Is this stated?

ciarmcom commented 5 years ago

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-341

hugueskamba commented 5 years ago

As there is no direct CMSIS equivalent for most RTX functionalities used in mbed_error.c, it is not worth changing only the ones available (osThreadGetId(), osThreadGetStackSize(), osThreadGetStackSpace()) and still have some RTX references. I discussed this with @BartSX

adbridge commented 3 years ago

As there has been no movement on this issue for the last 2 years I am closing it. Please re-open if it is still a valid issue.