Re: [PATCH 3/3] rv: Add explicit lockdep context for reactors

On Tue, Oct 14, 2025 at 12:22:06PM +0200, Gabriele Monaco wrote:

On Tue, 2025-10-14 at 11:46 +0200, Thomas Weißschuh wrote:

quoted

On Tue, Oct 14, 2025 at 09:38:09AM +0200, Nam Cao wrote:

quoted

The reactors are invoked in tracepoints' handlers, thus they must not
trigger another tracepoint, otherwise we may be stuck in an infinite loop.
(this is why preempt_enable_notrace() exists alongside preempt_enable()).

(...)

quoted

quoted

I'm not familiar with the internal lockdep. But I think these would
trigger trace_lock_acquire() and trace_lock_release().

Indeed. Right now no monitor attaches to those tracepoints. We could
prevent monitors from attaching to certain "well-known" tracepoints.
But then we still need to manually track which those are, which is ugly.
Or we move the invocation of the reactor to a workqueue/task_work.

I'm afraid also workqueues might open a rabbit-hole (waking up a task fights
with locks in many scheduling tracepoints).
At a quick glance task_works also do some IPI/wakeups that are traced.
If I get it correctly we are looking for something absolutely lock-free/trace-
free, I can't really think of much at the moment, maybe abusing RCU callbacks
but those would have their set of problems too.

Agreed.

As much as it might be interesting to write monitors on lockdep tracepoints,
this seems challenging.

We could opt for a foolproof Kconfig solution and prevent reactors if lockdep is
active (leaving only the error tracepoints that are hopefully still safe).

I can't follow here. lockdep can indicate problems, but it should not introduce
problems on its own. So preventing the usage together with lockdep would be the
proverbial head in the sand. If the tracepoints called by lockdep are an issue
then we would just not call into lockdep in the first place. lockdep triggering
these tracepoints should not be an issue in practice. I don't see a bulletproof
way to prevent a tracepoint handler from calling another tracepoint, except
maybe extending lockdep to also track that.


Thomas