Re: [RFC PATCH 7/9] rv: Retry when da monitor detects race conditions

[Nam Cao <namcao@linutronix.de>]

On Fri, Apr 04, 2025 at 10:45:20AM +0200, Gabriele Monaco wrote:
> DA monitor can be accessed from multiple cores simultaneously, this is
> likely, for instance when dealing with per-task monitors reacting on
> events that do not always occur on the CPU where the task is running.
> This can cause race conditions where two events change the next state
> and we see inconsistent values. E.g.:
> 
>   [62] event_srs: 27: sleepable x sched_wakeup -> running (final)
>   [63] event_srs: 27: sleepable x sched_set_state_sleepable -> sleepable
>   [63] error_srs: 27: event sched_switch_suspend not expected in the state running
> 
> In this case the monitor fails because the event on CPU 62 wins against
> the one on CPU 63, although the correct state should have been
> sleepable, since the task get suspended.
> 
> Detect if the current state was modified by using try_cmpxchg while
> storing the next value. If it was, try again reading the current state.
> After a maximum number of failed retries, react as if it was an error
> with invalid current state (we cannot determine it).
> 
> Monitors where this type of condition can occur must be able to account
> for racing events in any possible order, as we cannot know the winner.

Is locking not simpler? I understand raw_spin_lock() doesn't work because
it steps on some tracepoints. But how about adding something like
raw_spin_lock_notrace()?

static inline bool raw_spin_lock_notrace(raw_spinlock_t *lock)
{
	preempt_disable_notrace(); //probably not required, tracepoint handlers do this already

	if (!do_raw_spin_trylock(lock))
		do_raw_spin_lock(lock);
}

My LTL series theoretically also has this problem, but I have never got it
during testing yet. We should use the same solution for both DA and LTL.

Also, can you please Cc me in your RV patches?

Best regards,
Nam