Gabriele Monaco <[email protected]> writes: > From: Wen Yang <[email protected]> > > da_monitor_start() set monitoring=1 before calling da_monitor_init_hook(), > may racing with the sched_switch handler: > > da_monitor_start() sched_switch handler > ------------------------- --------------------------------- > da_mon->monitoring = 1; > if (da_monitoring(da_mon)) /* true */ > ha_start_timer_ns(...); > /* hrtimer->base == NULL, crash */ > da_monitor_init_hook(da_mon); > /* hrtimer_setup() sets base */ > > Fix the ordering and pair with release/acquire semantics: > > da_monitor_init_hook(da_mon); > smp_store_release(&da_mon->monitoring, 1); /* da_monitor_start() */ > return smp_load_acquire(&da_mon->monitoring); /* da_monitoring() */ > > On ARM64 a plain STR + LDR does not form a release-acquire pair, so > the load can observe monitoring=1 while hrtimer->base is still NULL. > The plain accesses are also data races under KCSAN. > > Use WRITE_ONCE for the monitoring=0 store in da_monitor_reset() to > cover the reset path. > > Fixes: 792575348ff7 ("rv/include: Add deterministic automata monitor > definition via C macros") > Signed-off-by: Wen Yang <[email protected]> > Reviewed-by: Gabriele Monaco <[email protected]> > Signed-off-by: Gabriele Monaco <[email protected]>
Looks correct to me. Reviewed-by: Nam Cao <[email protected]> Wen, I am curious, how did you find this issue? Nam
