-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Still needed for STM32CubeIDE v1.7.0 and later? #10
Comments
Hi Guido - I haven't reviewed this carefully, but some of it looks daft
or worse.
Malloc in an ISR is never, ever, ever a good idea, WTF? Are they still
flogging crap drivers that do that?
You CANNOT "reenter a critical section" in an ISR; why are they talking
about recursive mutex?
I fear morons stuck in an "/infinity loop/".
Aaarrrgggg.....
Really, I would take a VERY careful look before using any of this, sorry...
Best Regards, Dave
PS: We ditched an STM32-based project because their driver quality was
so poor.
It was too expensive to fix it; cheaper to move to an alternative product.
…On 2/20/2023 12:03 PM, Guido Mocken wrote:
At the time of writing, the current version of STM32CubeIDE is
v.1.11.2. Its user manual has a section 2.7 stating:
/*Thread-safe wizard for empty projects and CDTTM projects*
STM32CubeIDE includes a thread-safe wizard to generate files to
support the use of resources that can be updated by application code
and interrupts or when using a real-time operating system./
In this wizard one can select among several strategies, which are
described as follows in the manual:
1.
/User defined thread-safe implementation.
User defined solution for handling thread-safety.
NOTE: The stubs in stm32_lock_user.h needs to be implemented to gain
thread-safety./
2.
/Allow lock usage from interrupts.
This implementation will ensure thread-safety by disabling all
interrupts
during e.g. calls to malloc.
NOTE: Disabling all interrupts creates interrupt latency which
might not be desired for this application!/
3.
/Deny lock usage from interrupts.
This implementation assumes single thread of execution.
Thread-safety dependent functions will enter an infinity loop
if used in interrupt context./
4.
/Allow lock usage from interrupts. Implemented using FreeRTOS locks.
This implementation will ensure thread-safety by entering RTOS ISR
capable
critical sections during e.g. calls to malloc.
By default this implementation supports 2 levels of recursive locking.
Adding additional levels requires 4 bytes per lock per level of RAM.
NOTE: Interrupts with high priority are not disabled. This implies
that the lock is not thread-safe from high priority interrupts!/
5.
/Deny lock usage from interrupts. Implemented using FreeRTOS locks.
This implementation will ensure thread-safety by suspending all tasks
during e.g. calls to malloc.
NOTE: Thread-safety dependent functions will enter an infinity loop
if used in interrupt context./
I do not remember such options being present in releases earlier than
1.7.0, whose release notes and erratum list "Thread-safe malloc
solution" for the first time.
While the above sounds very promising, does it really mean that Dave's
excellent FreeRTOS helpers are no longer needed?
While they work very well, setup and usage together with
STM32CubeIDE's code generation is somewhat tiring, so it would be nice
to have an equivalent one-click replacement.
—
Reply to this email directly, view it on GitHub
<#10>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBYXZC3ACBIFKXSE5QN5XLWYOPX7ANCNFSM6AAAAAAVCDOHKY>.
You are receiving this because you are subscribed to this
thread.Message ID: ***@***.***>
--
Dave Nadler, USA East Coast voice (978) ***@***.***, Skype
Dave.Nadler1
|
Thanks for the quick reply, Dave. |
I've had a look at the code generated by STM32CubeMX 6.7.0 I've specifically only looked at what is generated under Project Manager -> Thread-safe Locking Strategy -> FreeRTOS Strategy 4 & 5. I've also used the CMSIS_V2 FreeRTOS option Strategy 4: as they describe it disables lower priority interrupts during an ISR when you call something like malloc. The issue with this is as they point out if you call malloc in an ISR and a higher priority interrupt comes in and also calls it the whole thing fails. I honestly can't see a use for this. It could easily introduce hard to track bugs. They do openly say "Using C library functions, which may need synchronization, at the same time from both the main application (or thread) and from the ISR, is not recommended." so I don't think they are endorsing this approach but it does seem odd to include it. Strategy 5: this disables "tasks" when something like malloc is called. On a side note they implement the __retarget_lock functions. These are needed for things like stdio and FILE operations, not really relevant to bare-metal but maybe someone will use it occasionally. They use newlib's malloc for the locks though which is odd, especially under FreeRTOS. There's a better version of that here which uses the correct primitives. Apparently this can use a fair amount of RAM though as FreeRTOS mutexs are quite beefy It's worth noting I hit a few bugs with the code compilation for a STM32G061C6T device though.
|
Upon closer inspection of their code and Application Note, their variants #4 and #5 seem to roughly correspond to the behavior one gets from Dave's code with MALLOCS_INSIDE_ISRs either defined or undefined, respectively. (These are just my observations, no actual test was performed.) |
Thanks for the update.
Sounds like STM still have a huge mess...
…On 2/21/2023 6:37 AM, Xenoamor wrote:
I've had a look at the code generated by STM32CubeMX 6.7.0
I've specifically only looked at what is generated under Project
Manager -> Thread-safe Locking Strategy -> Allow/deny lock usage from
interrupts. I've also used the CMSIS_V2 FreeRTOS option
Strategy 4: as they describe it disables lower priority interrupts
during an ISR when you call something like malloc. The issue with this
is as they point out if you call malloc in an ISR and a higher
priority interrupt comes in and also calls it the whole thing fails. I
honestly can't see a use for this. It could easily introduce hard to
track bugs. They do openly say "Using C library functions, which may
need synchronization, at the same time from both the main application
(or thread) and from the ISR, is not recommended." so I don't think
they are endorsing this approach but it does seem odd to include it.
Strategy 5: this disables "tasks" when something like malloc is called.
I think this is very comparable to this code? There's no _sbrk_r
implementation, I'm unsure if a _sbrk_r function is required or if
newlib brings a suitable default with it. Personally I always override
malloc and friends to point to the FreeRTOS versions. You can
implement an "Error_Handler()" which gets called if you try to lock
during an interrupt else it will just sit in an infinite loop.
On a side note they implement the __retarget_lock functions. These are
needed for things like stdio and FILE operations, not really relevant
to bare-metal but maybe someone will use it occasionally. They use
newlib's malloc for the locks though which is odd, especially under
FreeRTOS. There's a better version of that here
<https://gist.github.com/thomask77/65591d78070ace68885d3fd05cdebe3a>
which uses the correct primitives.
It's worth noting I hit a few bugs with the code compilation for a
STM32G061C6T device though.
1. It was supposed to warn me to enable configUSE_NEWLIB_REENTRANT in
the cube but didn't. Looks like it does at compile time though
2. It set both -DSTM32_THREAD_SAFE_STRATEGY=4 and
-DSTM32_THREAD_SAFE_STRATEGY=5 at the same time in my makefile
when I wanted strategy 5. This is fairly nasty as it actually
makes it use strategy 4 which is the awful one
—
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBYXZEDVO4S7D24V5PZENDWYSSHTANCNFSM6AAAAAAVCDOHKY>.
You are receiving this because you commented.Message ID:
***@***.***>
--
Dave Nadler, USA East Coast voice (978) ***@***.***, Skype
Dave.Nadler1
|
Last I looked (couple years back) they STM drivers called malloc (USB etc).
Anyway, thanks for the update, sounds like STM still have a huge mess...
…On 2/21/2023 7:00 AM, Guido Mocken wrote:
Upon closer inspection of their code and Application Note, their
variants #4 <#4>
and #5 <#5> seem to
roughly correspond to the behavior one gets from Dave's code with
MALLOCS_INSIDE_ISRs either defined or undefined, respectively.
The recursive mutex in variant #4
<#4> however is
still a bit of a mystery to me. If, as they say, high-priority ISRs
are strictly not allowed to call malloc(), it should not hurt, while
allowing recursive locking inside low-priority ISRs. I do not know if
this feature is needed.
Also, their new _sbrk() no longer refers to the current stack pointer,
but only to constant linker symbol definitions.
(These are just my observations, no actual test was performed.)
—
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBYXZHN5D4TY7LRLE6DPADWYSU5BANCNFSM6AAAAAAVCDOHKY>.
You are receiving this because you commented.Message ID:
***@***.***>
--
Dave Nadler, USA East Coast voice (978) ***@***.***, Skype
Dave.Nadler1
|
I cannot comment on previous or current USB drivers, as I only ever used I2C, SPI, UART, GPIO HAL-drivers together with your MALLOCS_INSIDE_ISR-undefined variant, but I might now go ahead and try out their variant 5. If it fails, I know that I have a fallback option here. |
You have to do the analysis, not just "wait til it fails" when
discussing race and concurrency conditions...
Be careful out there!
On 2/21/2023 9:26 AM, Guido Mocken wrote:
I cannot comment on previous or current USB drivers, as I only ever
used I2C, SPI, UART, GPIO HAL-drivers together with your
MALLOCS_INSIDE_ISR-undefined variant, but I might now go ahead and try
out their variant 5. If it fails, I know that I have a fallback option
here.
--
Dave Nadler, USA East Coast voice (978) ***@***.***, Skype
Dave.Nadler1
|
The latest CubeMX keeps |
At the time of writing, the current version of STM32CubeIDE is v.1.11.2. Its user manual has a section 2.7 stating:
Thread-safe wizard for empty projects and CDTTM projects
STM32CubeIDE includes a thread-safe wizard to generate files to support the use of resources that can be updated by application code and interrupts or when using a real-time operating system.
In this wizard one can select among several strategies, which are described as follows in the manual:
User defined thread-safe implementation.
User defined solution for handling thread-safety.
NOTE: The stubs in stm32_lock_user.h needs to be implemented to gain
thread-safety.
Allow lock usage from interrupts.
This implementation will ensure thread-safety by disabling all interrupts
during e.g. calls to malloc.
NOTE: Disabling all interrupts creates interrupt latency which
might not be desired for this application!
Deny lock usage from interrupts.
This implementation assumes single thread of execution.
Thread-safety dependent functions will enter an infinity loop
if used in interrupt context.
Allow lock usage from interrupts. Implemented using FreeRTOS locks.
This implementation will ensure thread-safety by entering RTOS ISR capable
critical sections during e.g. calls to malloc.
By default this implementation supports 2 levels of recursive locking.
Adding additional levels requires 4 bytes per lock per level of RAM.
NOTE: Interrupts with high priority are not disabled. This implies
that the lock is not thread-safe from high priority interrupts!
Deny lock usage from interrupts. Implemented using FreeRTOS locks.
This implementation will ensure thread-safety by suspending all tasks
during e.g. calls to malloc.
NOTE: Thread-safety dependent functions will enter an infinity loop
if used in interrupt context.
I do not remember such options being present in releases earlier than 1.7.0, whose release notes and erratum list "Thread-safe malloc solution" for the first time.
While the above sounds very promising, does it really mean that Dave's excellent FreeRTOS helpers are no longer needed?
While they work very well, setup and usage together with STM32CubeIDE's code generation is somewhat tiring, so it would be nice to have an equivalent one-click replacement.
The text was updated successfully, but these errors were encountered: