Re: [PATCH v2 19/22] rv: Add rtapp_pagefault monitor

Gabriele Monaco
2025-04-15T12:47:27Z

On Tue, 2025-04-15 at 14:38 +0200, Nam Cao wrote:
> On Tue, Apr 15, 2025 at 02:31:43PM +0200, Gabriele Monaco wrote:
> > On Fri, 2025-04-11 at 09:37 +0200, Nam Cao wrote:
> > > +static int __init register_pagefault(void)
> > > +{
> > > + rv_register_monitor(&rv_pagefault, &rv_rtapp);
> > > + return 0;
> >
> > Any reason why you aren't returning the error value from the
> > monitor
> > registration?
>
> Copy-paste from dot2k :P

Mmh, you're right! All other monitors are broken in this sense..

>
> > Other than that, the monitor seems neat and reasonably easy to
> > generate.
> >
> > May not be necessary in this series, but try to keep compatibility
> > with
> > the userspace RV tool as well, you need to have some special case
> > in
> > its tracing components because fields are not matching:
> >  # rv mon sleep -t
> >          rcuc/11-108      [011] event <CANT FIND FIELD
> > final_state>
> > (null) x (null)                   -> (null)                   Y
> >          rcuc/11-108      [011] event <CANT FIND FIELD
> > final_state>
> > (null) x (null)                   -> (null)                   Y
> >       ktimers/11-109      [011] event <CANT FIND FIELD
> > final_state>
> > (null) x (null)                   -> (null)                   Y
>
> I have this userspace RV tool in my "later" TODO list, if that's
> okay.
>
> Honestly, I haven't looked at what it does yet. perf already does
> what I
> need.

Yeah, no rush, the tool is mostly for enabling the monitor and reactors
in a single command, the rest (tracing) you can already do with perf,
trace-cmd and friends, after enabling the monitor manually, of course.

We may even consider integrating RV in other tools instead of
maintaining a separate one, but that's for another day.

Thanks,
Gabriele