-
Notifications
You must be signed in to change notification settings - Fork 236
Description
Describe the bug
The CFE_ES_PerfLogAdd() routine uses OS_IntLock() in an attempt to get exclusive access to a common global data structure to record performance metrics/state. This is insufficient.
The OS_IntLock function is generally not implemented on SMP, and even if it is, it probably only affects the current core. Either way, it will not provide exclusivity, because the other cores can still access the data even when interrupts are disabled.
This function is also a no-op in the POSIX OSAL.
To Reproduce
Enable performance monitoring on a POSIX system and observe that occasionally samples occur in the log out of order or otherwise appear corrupted. This is likely due to concurrent writes to the same entry related to insufficient locking.
Expected behavior
The function should use some form of primitive that actually does provide exclusivity between threads (such as a mutex/spinlock) and not an interrupt lock.
Code snips
cFE/fsw/cfe-core/src/es/cfe_es_perf.c
Lines 439 to 440 in 2b27dfc
| /* disable interrupts to guarentee exclusive access to the data structures */ | |
| IntFlags = OS_IntLock(); |
System observed on:
Ubuntu 18.04 LTS 64-bit
Reporter Info
Joseph Hickey, Vantage Systems, Inc.