Re: [PATCH v7 00/14] rv: Add task latency over budget RV monitor

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-10-12 at 03:39 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> New in this revision:
>   - Patches 1 and 2: the HA environment lifecycle and timer fixes, in
>     that order (see above).  Patch 3 adds KUnit coverage for both.
> 
>   - Patch 4 (was 1): the HA timer teardown fix, found while auditing
>     tlob's own teardown.  The gap is in the framework; a monitor cannot
>     close it.  Patch 5 adds KUnit coverage, and Patch 8 splits the
>     per-object teardown hook out of the tlob patch.
> 
>   - Dropped detail_env_tlob and the per-state accounting.  Which phase
>     consumed the budget is now derived from event_tlob in user space;
>     monitor_tlob.rst documents the recipe.  This removes the per-record
>     lock and the accumulator from the hot path, and tlob no longer
>     deviates from "run a model and spot violations" (Gabriele, v2).

Very well.
You may find [1] of interest as it has some overlap with that accounting, aiming
to introduce stats into the DA infrastructure. It probably isn't a direct match
and only targets per-cpu for now.

By the way there has been a running discussion about keeping the C files for the
monitors as close as possible to the DOT representation, with a long-term goal
to extend the DOT to represent everything and have logic represented there too
(at least simple trace filters).

Now I haven't been following this religiously either and some things are simply
too hard to be represented there. But I'd like you to keep it in mind when
writing your model, since yours deviates considerably.

For example I'm still not convinced by the fact that you maintain a parallel
state (OPEN/CLOSE/RETIRED). You do a lot of error reporting in tlob_start_task()
that is mainly discarded (return value ignored). What prevents you from allowing
the start event in running and just not allocate if you have already a target?

I'd really prefer to have the complexity moved to the model. I'm probably
missing some background here, but I see an open/closed window as something
mostly matching the monitor's state, while the retired state as the result of a
race with cleanup.
Where are you seeing such races? When the monitor is stopped while a window is
still open? da_extra_cleanup() is supposed to run after everything else settled
down, if you have evidence this isn't the case, let's discuss about that first.

If you could drop these states, you'd also no longer need tlob_reset_notify()
and many other things will get simplified and closer to what rgven gives you.

And it's also good to keep everything rvgen gives you as is
(ha_verify_invariants() and friends). Otherwise you'd never tell your model
deviated from the code.

These are general guidelines, the C files cannot yet be a 1-to-1 product of the
model, but shouldn't be a completely different story either.

> 
>   - The model gives stopped a self-loop for every scheduler event, and the
>     sched handlers call da_handle_event() unconditionally.  The
>     source-side gate that skipped events while stopped is gone; the logic
>     lives in the model (Gabriele, v6).
> 
>   - tlob_sym.c (a 235-line ELF parser) is replaced by a sym_offset()
>     shell helper built on readelf (Gabriele, v3).

Great! Let's keep the code to maintain to a minimum.

> 
>   - tlob_preempt_work, identical to tlob_busy_work, is dropped and the
>     busy-wait body is shared (Gabriele, v3).
> 
>   - The KUnit and DOT commit messages were rewritten as requested
>     (Gabriele, v6).
> 
>   - Dropped three patches that no longer stand on their own: "rv: Fix
>     ha_invariant_passed_ns silent bypass of invariant check" (Patch 1
>     makes it unnecessary), "rv: Check the decoded fields in the tlob
>     parser tests" and "selftests/verification: Fix up the tlob selftests"
>     (folded into the patches they fix, so each patch is complete on its
>     own).
> 

Thanks,
Gabriele

[1] -
https://lore.kernel.org/lkml/20261020095512.863754-1-tobias.schaffner@siemens.com

> Also folded into the patches they belong to, from earlier review rounds:
> the DA_MON_POOL_SIZE dual-macro simplification and DA_ALLOC_MANUAL
> (Gabriele), the invariant-check rewrite (sashiko-bot), and the tlob
> teardown/error-path fixes: security_locked_down(LOCKDOWN_TRACEFS) in
> tlob_monitor_open(), the single tlob_record_state word with
> TLOB_RETIRED as the only licence to free, tlob_add_uprobe()'s rollback
> running the full nosync/quiesce/sync/reap sequence, and
> tlob_remove_all_uprobes() holding tlob_uprobe_mutex across the sync and
> the reap.
> 
> Testing
> =======
> Environment: x86_64, CONFIG_PREEMPT_RT=y, kernel built with virtme-ng
> (vng --build), selftests built on the host and run inside the VM via
> `ftracetest --rv ../verification` (the verificationtest-ktap entry
> point, with logs redirected to /tmp):
> - KUnit: rv_mon suite (CONFIG_RV_MONITORS_KUNIT_TEST), rv_test_tlob case
>   covering the uprobe-line parser
> - 11 ftrace-style selftests: uprobe_bind, uprobe_violation,
>   uprobe_no_event, uprobe_multi, uprobe_multi_instance,
>   uprobe_detail_{running,sleeping,waiting}, uprobe_restart,
>   uprobe_overlap_register, uprobe_restart_violation
> Both suites pass.  The pre-existing rv_* tests in the same directory
> and the rest of the rv_mon KUnit suite (nomiss, sco, sssw, sts, opid,
> pagefault, sleep) keep passing as well.
> 
> v6: https://lore.kernel.org/all/cover.1787243842.git.wen.yang@linux.dev/
> v4: https://lore.kernel.org/all/cover.1783524627.git.wen.yang@linux.dev/
> v3: https://lore.kernel.org/all/cover.1780847473.git.wen.yang@linux.dev/
> v2: https://lore.kernel.org/all/cover.1778522945.git.wen.yang@linux.dev/
> v1: https://lore.kernel.org/all/cover.1776020428.git.wen.yang@linux.dev/
> 
> Gabriele Monaco (1):
>   rv: Force environment reset action on HA monitor start
> 
> Wen Yang (13):
>   rv: Fix ha_monitor timer and invariant underflow
>   rv: Add a KUnit test for the HA timer underflow
>   rv: Disarm HA timers on the storage free path
>   rv: Add a KUnit test for the storage free path
>   rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
>   rv: Introduce DA_MON_ALLOCATION_STRATEGY
>   rv: Add per-object teardown hook da_extra_cleanup
>   rv: Add generic uprobe infrastructure for RV monitors
>   rv: Add tlob model DOT file
>   rv: Add tlob hybrid automaton monitor
>   rv: Add KUnit tests for the tlob monitor
>   selftests/verification: Add tlob selftests
>   selftests/ftrace: Walk up parent dirs to find test.d/functions
> 
>  Documentation/trace/rv/index.rst              |    1 +
>  Documentation/trace/rv/monitor_tlob.rst       |  154 +++
>  include/rv/da_monitor.h                       |  154 ++-
>  include/rv/ha_monitor.h                       |   61 +-
>  include/rv/rv_uprobe.h                        |   35 +
>  kernel/trace/rv/Kconfig                       |    5 +
>  kernel/trace/rv/Makefile                      |    2 +
>  kernel/trace/rv/monitors/nomiss/nomiss.c      |    2 +-
>  kernel/trace/rv/monitors/opid/opid.h          |    1 +
>  kernel/trace/rv/monitors/tlob/Kconfig         |   12 +
>  kernel/trace/rv/monitors/tlob/tlob.c          | 1049 +++++++++++++++++
>  kernel/trace/rv/monitors/tlob/tlob.h          |  142 +++
>  kernel/trace/rv/monitors/tlob/tlob_kunit.c    |  151 +++
>  kernel/trace/rv/monitors/tlob/tlob_kunit.h    |   18 +
>  kernel/trace/rv/monitors/tlob/tlob_trace.h    |   20 +
>  kernel/trace/rv/rv_monitors_test.c            |  162 +++
>  kernel/trace/rv/rv_trace.h                    |    1 +
>  kernel/trace/rv/rv_uprobe.c                   |   56 +
>  tools/testing/selftests/ftrace/ftracetest     |   19 +-
>  .../testing/selftests/verification/.gitignore |    1 +
>  tools/testing/selftests/verification/Makefile |    4 +
>  tools/testing/selftests/verification/config   |    3 +
>  .../selftests/verification/test.d/functions   |    9 +
>  .../test.d/tlob/run_tlob_tests.sh             |   32 +
>  .../verification/test.d/tlob/uprobe_bind.tc   |   47 +
>  .../test.d/tlob/uprobe_breakdown.tc           |  136 +++
>  .../test.d/tlob/uprobe_disable_live.tc        |   79 ++
>  .../verification/test.d/tlob/uprobe_multi.tc  |   58 +
>  .../test.d/tlob/uprobe_multi_instance.tc      |   65 +
>  .../test.d/tlob/uprobe_no_event.tc            |   17 +
>  .../test.d/tlob/uprobe_overlap_register.tc    |   66 ++
>  .../test.d/tlob/uprobe_restart.tc             |   66 ++
>  .../test.d/tlob/uprobe_restart_violation.tc   |   48 +
>  .../test.d/tlob/uprobe_unbind_live.tc         |   65 +
>  .../test.d/tlob/uprobe_violation.tc           |   53 +
>  .../selftests/verification/tlob_target.c      |  127 ++
>  tools/verification/models/tlob.dot            |   27 +
>  tools/verification/rvgen/rvgen/dot2c.py       |    2 +
>  38 files changed, 2930 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/trace/rv/monitor_tlob.rst
>  create mode 100644 include/rv/rv_uprobe.h
>  create mode 100644 kernel/trace/rv/monitors/tlob/Kconfig
>  create mode 100644 kernel/trace/rv/monitors/tlob/tlob.c
>  create mode 100644 kernel/trace/rv/monitors/tlob/tlob.h
>  create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.c
>  create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.h
>  create mode 100644 kernel/trace/rv/monitors/tlob/tlob_trace.h
>  create mode 100644 kernel/trace/rv/rv_uprobe.c
>  create mode 100755
> tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_breakdown.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_disable_live.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_multi.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_multi_instance.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_no_event.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_overlap_register.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_restart.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_restart_violation.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_unbind_live.tc
>  create mode 100644
> tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tc
>  create mode 100644 tools/testing/selftests/verification/tlob_target.c
>  create mode 100644 tools/verification/models/tlob.dot