hujun260 commented on code in PR #18284:
URL: https://github.com/apache/nuttx/pull/18284#discussion_r2753792407
##########
sched/clock/clock_sched_ticks.c:
##########
@@ -62,13 +64,13 @@ static volatile clock_t g_system_ticks =
INITIAL_SYSTEM_TIMER_TICKS;
void clock_increase_sched_ticks(clock_t ticks)
{
+ irqstate_t flags;
+
/* Increment the per-tick scheduler counter */
-#ifdef CONFIG_SYSTEM_TIME64
- atomic64_fetch_add((FAR atomic64_t *)&g_system_ticks, ticks);
-#else
- atomic_fetch_add((FAR atomic_t *)&g_system_ticks, ticks);
-#endif
+ flags = write_seqlock_irqsave(&g_system_tick_lock);
Review Comment:
> But you attempted an atomic optimization in #15414, so atomic64 didn't
take effect in your changes, did it? Didn't you check the generated
instructions after making the modifications?
The implementation of atomic64 on 32-bit architectures is located in
libs/libc/machine/arch_atomic.c. It only works on single-core systems and fails
in SMP scenarios.
> **g_system_ticks** is only updated in the interrupt context, so:
g_system_ticks can update in thread context in clock_resynchronize
> a. This implementation works fine in AMP mode.
In an AMP scenario, when CONFIG_SYSTEM_TIME64 is enabled, the current
implementation does not guarantee the execution order of:
```
verify = g_system_ticks;
sample = g_system_ticks;
```
> b. No issues arise on 64-bit hardware in SMP mode.
yes , However, this will cause a performance degradation in
clock_get_sched_ticks
<img width="1460" height="271" alt="image"
src="https://github.com/user-attachments/assets/c339bda2-08af-4bbc-a579-f59e74f64467";
/>
> Therefore, your commit needs to add a conditional check for SMP with
32-bit hardware.
This would result in rather complex macro control.
> Because the current implementation is more streamlined on AMP and 64bit
SMP device.
When CONFIG_SYSTEM_TIME64=y, the implementation logic of
clock_get_sched_ticks becomes more complex.
<img width="1460" height="271" alt="image"
src="https://github.com/user-attachments/assets/c339bda2-08af-4bbc-a579-f59e74f64467";
/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
