All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: Gabriele Monaco <gmonaco@redhat.com>
Cc: Nam Cao <namcao@linutronix.de>,
	linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 00/14] rv: Add task latency over budget RV monitor
Date: Tue, 20 Oct 2026 03:18:48 +0800	[thread overview]
Message-ID: <6a8b5d26-37cc-4a8f-ab1d-0df2c2e6c2d3@linux.dev> (raw)
In-Reply-To: <3eb0948c9ca67f64b3b311f823cd0f7628033c68.camel@redhat.com>



On 10/12/26 19:58, Gabriele Monaco wrote:
> On Mon, 2026-10-12 at 03:39 +0800, wen.yang@linux.dev wrote:
>> From: Wen Yang <wen.yang@linux.dev>
>>
>> This series introduces tlob (task latency over budget), a per-task
>> hybrid automaton RV monitor that measures elapsed wall-clock time across
>> a user-delimited code section and emits an error when the elapsed time
>> exceeds a configurable budget.
> 
> Hi Wen,
> 
Hi Gabriele,

Thanks for the context.

> there's been a conversation at the LPC conference about completely migrating RV
> to BPF.
> While we aren't sure of its feasibility yet, and it will take time, it's
> something to keep in mind for new contributions.
> 

Understood, and I have been following that direction.  I spent the last
few days reading the v2 "rv: Add support for BPF monitors" series in
detail, so the comparison below is against that code rather than the
general idea.


> First of all, would dropping RV in-kernel support negatively affect your current
> workflow?
>
Yes, it would.  Some concrete points, all from the current tree.
(a) Event ABI.  event_da_monitor / error_env_* in
     kernel/trace/rv/rv_trace.h are plain tracepoints, so trace-cmd,
     perf and anything else reading tracefs consume them today with no
     extra userspace.  The BPF series delivers events through a ring
     buffer read by the rv tool, so the two backends are not
     interchangeable for existing consumers.

(b) Build and deployment.  On the images we target, BPF is often not
     compiled in or not permitted:
       CONFIG_BPF_SYSCALL            default n
       CONFIG_BPF_EVENTS             needs BPF_SYSCALL
       CONFIG_DEBUG_INFO_BTF         needed for CO-RE; depends on
                                     !DEBUG_INFO_REDUCED and pahole
       LOCKDOWN_BPF_READ_KERNEL      disables the probe_read helpers
     Those are product constraints, not local build issues.  tracefs/RV
     works in all of these configurations.

     There is also a size cost: BPF implies CONFIG_DEBUG_INFO_BTF, and
     .BTF is resident in RO_DATA and kept in the boot image - about
     5 MB uncompressed on a full kernel, 1.2-1.6 MB compressed, plus
     roughly 25% per module with DEBUG_INFO_BTF_MODULES.  On a
     16-32 MB kernel partition that matters.

(c) Timing semantics.  tlob arms a per-task hrtimer with
     HRTIMER_MODE_REL_HARD.  bpf_timer is fixed to
     HRTIMER_MODE_REL_SOFT and the verifier states "bpf_timer callbacks
     are never sleepable" .  On PREEMPT_RT the callback runs in a 
threaded
     softirq, so the expiry time carries scheduler latency.
     For a budget monitor the expiry time is the output, so this is a
     semantic change, not just overhead.

     What a BPF tlob would look like once those exist:
       - model: still tlob.dot, with rvgen -b extended to the hybrid
         backend;
       - state: one record per task in BPF_MAP_TYPE_TASK_STORAGE (or
         the per-object hash map the series has today), holding the
         state, the binding id and the budget;
       - window: two uprobe programs attached to PATH:START and
         PATH:STOP, with the binding id and budget passed through the
         attach cookie or a small config map populated before load;
       - expiry: a bpf_timer in the same record, armed on state
         transitions and cancelled on STOP, submitting an error entry
         to the monitor's ringbuf;
       - output: the rv tool reads the ringbuf and prints it, and the
         reactor is linked in with bpf_linker as v2 already does.

     The DA engine and the ringbuf/reactor plumbing would carry over
     unchanged.  The three missing pieces are the uprobe
     attach/configuration path, the timer and env handling in
     da_monitor_bpf.h, and the hybrid backend in rvgen.

     None of this argues against migrating.  It argues that the BPF
     backend has to reach parity on (a)-(c) before the in-kernel path
     can be removed.

> About this series, completely converting it to BPF isn't trivial, I can keep on
> reviewing it and we could upstream it in-kernel, then convert it later (together
> with the work with other HA).

That is the order I would like to take.  The series is already split so
that a later conversion only replaces the tlob-specific part:

   1-5   framework fixes, independent of tlob and needed regardless:
         - HA environment reset on monitor start (Patch 1)
         - HA timer remainder / invariant underflow (Patch 2)
         - HA timer still queued on storage freed via RCU (Patch 4)
         3 and 5 are their KUnit coverage.
   6-8   small extension points tlob needs, but generic: overridable
         reset hook / EVENT_NONE_LBL, the per-object allocation
         strategy, and da_extra_cleanup.
   9-13  uprobe wrapper and tlob itself (DOT model, monitor, KUnit,
         verification selftests).
   14    ftracetest: find test.d/functions from a nested test
         directory.  Generic, not tlob-specific.

Could we merge the whole series now rather than staging it:
   - 1-5 fix live bugs and should not wait for an unrelated design
     decision;
   - the BPF work needs the model in tlob.dot and the tracepoint fields
     in rv_trace.h as a concrete reference now, rather than being
     designed twice;
   - 9-13 are self-contained - one monitor and its tests, plus no shared
     framework beyond the small hooks in 6-8 - so accepting them does
     not commit the framework to anything that would make the later
     conversion harder.

When the BPF backend reaches parity on (a)-(c), 9-13 are replaced
without touching 1-8 or 14.


> But I'd prioritise not affecting the in-kernel implementation much to keep it on
> par with the BPF one.
> 

Agreed, and that is why 6-8 are deliberately small and generic, and why
the monitor keeps the same model/event contract as the BPF path instead
of growing in-kernel-only semantics.

> I'd first like to focus on what should the rv userspace tool support for a
> monitor like yours to be useful. You'd have all reporting as stdout (or any BPF
> reactor you'd want to write), then we should probably support some way to set
> the uprobe location and threshold before loading.

Reporting already works: `rv mon -t tlob` creates a trace instance and
prints event_tlob / error_env_tlob through ikm_event_handler() and
ikm_error_handler(), so a BPF backend could feed the same output.

The missing piece is pre-enable configuration.  Proposal:
   rv mon tlob --uprobe PATH:START:STOP[:threshold=NS]   (repeatable)
   rv mon tlob --list-uprobes
   rv mon tlob --budget NS            default for bindings without one
   rv mon tlob --summary              per-pid counts instead of transitions
The tool would write the monitor file before enabling.  tracefs stays
the low-level path for scripts and tests, as uprobe_events coexists
with perf.


> Would you really need dynamic uprobe addition?
> 
Minimum: bindings configured before enable.  That covers the normal
case, including restarting the target, because a binding is per
path+offset, not per pid.

Live add/remove is what makes the code large: quiesce/reap of in-flight
windows, the TLOB_RETIRED claim, concurrent add/remove versus handlers,
and shadowed-binding reporting when two ranges match one task.  If you
want the smaller interface, I will require configuration while the
monitor is disabled and keep only the teardown needed for disable and
unload with live tasks; uprobe_unbind_live, uprobe_disable_live and
uprobe_multi_instance then move to the tool's restart flow.


> What do you think?
> 

If you're OK with it, I'd suggest merging the series in-kernel now, with 
1-8 and 14 kept as the shared infrastructure and 9-13 acting as the 
reference implementation the BPF backend is measured against. I'd be 
glad to help with the uprobe and timed-HA pieces on the BPF side once 
they're on the table.


Thanks,
Wen



  reply	other threads:[~2026-10-19 19:19 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-11 19:39 [PATCH v7 00/14] rv: Add task latency over budget RV monitor wen.yang
2026-10-11 19:39 ` [PATCH v7 01/14] rv: Force environment reset action on HA monitor start wen.yang
2026-10-11 19:39 ` [PATCH v7 02/14] rv: Fix ha_monitor timer and invariant underflow wen.yang
2026-10-11 19:39 ` [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow wen.yang
2026-10-11 19:52   ` sashiko-bot
2026-10-12  7:47   ` kernel test robot
2026-10-12  9:27   ` kernel test robot
2026-10-12 11:18   ` kernel test robot
2026-10-11 19:39 ` [PATCH v7 04/14] rv: Disarm HA timers on the storage free path wen.yang
2026-10-11 19:51   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 05/14] rv: Add a KUnit test for " wen.yang
2026-10-11 19:50   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 06/14] rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable wen.yang
2026-10-11 19:53   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 07/14] rv: Introduce DA_MON_ALLOCATION_STRATEGY wen.yang
2026-10-11 19:57   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 08/14] rv: Add per-object teardown hook da_extra_cleanup wen.yang
2026-10-11 19:54   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 09/14] rv: Add generic uprobe infrastructure for RV monitors wen.yang
2026-10-11 19:49   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 10/14] rv: Add tlob model DOT file wen.yang
2026-10-11 19:39 ` [PATCH v7 11/14] rv: Add tlob hybrid automaton monitor wen.yang
2026-10-11 19:58   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 12/14] rv: Add KUnit tests for the tlob monitor wen.yang
2026-10-11 19:39 ` [PATCH v7 13/14] selftests/verification: Add tlob selftests wen.yang
2026-10-11 19:51   ` sashiko-bot
2026-10-11 19:39 ` [PATCH v7 14/14] selftests/ftrace: Walk up parent dirs to find test.d/functions wen.yang
2026-10-12 11:58 ` [PATCH v7 00/14] rv: Add task latency over budget RV monitor Gabriele Monaco
2026-10-19 19:18   ` Wen Yang [this message]
2026-10-19 19:48     ` Steven Rostedt
2026-10-20 15:33       ` Gabriele Monaco
2026-10-20 15:30     ` Gabriele Monaco

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6a8b5d26-37cc-4a8f-ab1d-0df2c2e6c2d3@linux.dev \
    --to=wen.yang@linux.dev \
    --cc=gmonaco@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=namcao@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.