logging: Add implicit initialization when logger used before init #8742
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8742 +/- ##
==========================================
- Coverage 52.34% 52.31% -0.03%
==========================================
Files 195 195
Lines 24723 24741 +18
Branches 5139 5146 +7
==========================================
+ Hits 12941 12944 +3
- Misses 9708 9717 +9
- Partials 2074 2080 +6
Continue to review full report at Codecov.
|
subsys/logging/log_core.c
Outdated
There was a problem hiding this comment.
As the logging could be called from multiple threads that could be preempted, shouldn't the interface variable be atomic so that we would not be able call log_init() twice. By looking the code, it seems possible that log_init() could be initialized more than once.
There was a problem hiding this comment.
@jukkar good point, I've added following mechanism: logger can have 3 internal states: uninit'ed, init in progress, init'ed. each log API is atomic_inc state and only if previous state was uninit'ed it will perform initialization, if state is init in progress then log is dropped because we cannot do much since we interrupt log initialization.
I've added kindof state initial precheck before doing atomic incrementation for performance reasons.
Logger must support the case when API is used before logger is explicitly initialized. When logger API is called and logger is not yet initialized, it is initialized implicitly with dummy timestamp function (timestamp source may not be yet available). Signed-off-by: Krzysztof Chruscinski <[email protected]>
nordic-krch
force-pushed
the
logger_self_init
branch
from
77c7306 to
cff2617
Compare
There was a problem hiding this comment.
How this PR will cooperate with this one: #8704 ?
|
@pizi-nordic after recent updates in #8704 it will. |
pizi-nordic
left a comment
pizi-nordic
left a comment
There was a problem hiding this comment.
In my opinion we should just initialize the logger during system start-up. Then this change (which slightly degrades performance) will be not needed.
| } while (0) | ||
|
|
||
| #define LOG_COND_INIT() _COND_INIT(return) | ||
| #define LOG_COND_INIT_PRINTK() _COND_INIT(return 0) |
There was a problem hiding this comment.
Hiding return statement in macro is not a good idea. Please consider using anonymous functions to check the state and initialize logger.
|
@nashif regarding:
Do you know (or know who knows) where would be the best place to add that. In that case we could just add assert to log API checking if it is initialized to detect if somehow log is used before being initialized. |
The kernel/init.c looks like a good candidate to place such early init calls. |
4 participants
Logger must support the case when API is used before logger is explicitly
initialized. When logger API is called and logger is not yet initialized,
it is initialized implicitly with dummy timestamp function (timestamp
source may not be yet available).
Signed-off-by: Krzysztof Chruscinski [email protected]