[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXNkusXPp9MyRX9y@fedora>
Date: Fri, 23 Jan 2026 09:19:03 -0300
From: Wander Lairson Costa <wander@...hat.com>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: Steven Rostedt <rostedt@...dmis.org>, Nam Cao <namcao@...utronix.de>,
open list <linux-kernel@...r.kernel.org>,
"open list:RUNTIME VERIFICATION (RV)" <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH 18/26] rv/rvgen: add fill_tracepoint_args_skel stub to
ltl2k
On Thu, Jan 22, 2026 at 02:49:59PM +0100, Gabriele Monaco wrote:
> On Thu, 2026-01-22 at 10:10 -0300, Wander Lairson Costa wrote:
> > On Wed, Jan 21, 2026 at 02:53:03PM -0300, Wander Lairson Costa wrote:
> > > On Wed, Jan 21, 2026 at 02:57:02PM +0100, Gabriele Monaco wrote:
> > > > Mmh, this is a bit fishy though.
> > > > We the patch using the decorator seems fine, but highlights how this
> > > > method
> > > > isn't meant to be in Monitor if not all monitors use it..
> > > > Adding a stub here is just sweeping dust under the carpet.
> > > >
> > > > Here should probably keep the common part of fill_trace_h() in Monitor
> > > > (e.g.
> > > > replacing MODEL_NAME and other common things) and create specific
> > > > implementations in dot2k and ltl2k for what is not common while calling
> > > > the
> > > > super() counterpart for the rest.
> > > >
> > > > Does it make sense to you?
> > >
> > > Yes, that is exactly my idea. Since the patch series were getting too
> > > long and my brain too rot, I thought would be better addressing this in
> > > a following up patch series. But I can work in the next version if you
> > > are not ok with that approach.
>
> Good point, that can be a separate series so that we don't mix too many things,
> but I'd also separate the initial patch introducing the @not_implemented .
>
> > I gave more thought on this matter yesterday before bed. Maybe this
> > isn't a issue on the design. Some methods on Monitor might just have a
> > harmless default behavior. I look into it more closely for next the
> > round.
>
> Well, I believe that if a bunch of methods from the parent class don't need to
> be called and we have to create stubs just to avoid errors, those methods
> probably shouldn't be there in the first place.
>
> That's particularly valid for the Container class, that won't ever need to fill
> tracepoints and other stuff.
>
> Why fill_tracepoint_args_skel() is not required by LTL is an implementation
> detail, so that stub could even stay, in case future monitors are going to need
> the entire thing.
> Though I still find it cleaner to move that away too until there's a need for it
> shared in Monitor.
>
I didn't catch what is included in "that"...
> What do you think?
>
I agreed. fill_tracepoint_args_skel() makes sense in the Monitor class.
If a derived class doesn't need it, it is an implementation detail.
> Thanks,
> Gabriele
>
Powered by blists - more mailing lists