fix (tested on PCU, LV-BMS, HV-BMS and LCU)#594
fix (tested on PCU, LV-BMS, HV-BMS and LCU)#594victor-Lopez25 wants to merge 14 commits intodevelopmentfrom
Conversation
| Task& task = tasks_[bit_index]; | ||
| task.callback(); | ||
|
|
||
| SchedLock(); |
There was a problem hiding this comment.
No me mola mucho la verdad que el bitmap se limpie despues del callback, si hay problemas de rendimiento y el timer vuelve a hacerle set a la tarea la segunda ejecucion no se ejecuta, por lo demas no veo muchos mas problemas a hacerle clear despues del callback, se podria quedar asi pero solo digo
There was a problem hiding this comment.
Era por poner un solo lock, si lo ves mejor poner dos locks y así poder hacerlo antes lo puedo cambiar
|
|
||
| uint64_t Scheduler::get_global_tick() { | ||
| SchedLock(); | ||
| uint64_t val = global_tick_us_ + Scheduler_global_timer->CNT; |
There was a problem hiding this comment.
No entiendo porque esto necesita un lock, la lectura del CNT es atomica es imposible que esto se corrompa
There was a problem hiding this comment.
la lectura del CNT sí pero la lectura de global_tick_us_ no pq es uint64_t
| } | ||
| pop_front(); | ||
|
|
||
| SchedLock(); |
There was a problem hiding this comment.
tampoco tiene sentido hacerle disable al timer dentro del interrupt del timer
There was a problem hiding this comment.
Tengo que probar si realmente no hace falta, pq es una de las primeras cosas que probé ayer en las placas y la dejé
There was a problem hiding this comment.
Según la prueba de Boris en la LCU no hace falta pero voy a probarlo mañana (?) en las otras placas también
| return; | ||
| } | ||
|
|
||
| SchedLock(); |
There was a problem hiding this comment.
esto esta dentro del interrupt del timer no tiene sentido hacer disable al timer
| if (Scheduler_global_timer->CNT > current_interval_us_) [[unlikely]] { | ||
| uint32_t offset = Scheduler_global_timer->CNT - current_interval_us_; | ||
| Scheduler_global_timer->CNT = 0; | ||
| global_tick_us_ += offset; |
There was a problem hiding this comment.
esto esta simplemente mal, te estas adelantando en el tiempo, yo me guardaria otro valor que fuera accumulated offset y cuando esto pasa lo incrementas en la linea 307
ST-LIB Release Plan
Pending changes
|
victor-Lopez25
changed the title
2 participants
release: patch
summary: fix remaining scheduler race conditions