Re: [EXT] Re: [PATCH v5 1/1] devtools: add tracepoint check in checkpatch

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Aug 31, 2023 at 12:08 AM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday, 30 August 2023 18.24
> >
> > 21/08/2023 16:46, Morten Brørup:
> > > > From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
> > > > Sent: Monday, 21 August 2023 15.54

> >
> > In general, I wonder how much the check is useful compared to the
> > complexity.
> >
> >
> > > The bigger question is: Do we really want to change tracepoints in
> > functions from opt-in to opt-out?
> >
> > Yes that's the question: should traces be mandatory in some libs?
> >
> > > In my opinion, opt-in for trace is more appropriate.
> >
> > There was some work to add traces everywhere in few libs, so why not
> > maintaining this state?
>
> I still think it's a silly and burdening requirement, but I'm not against it for the libs where it is already a de facto standard.
>
> <irony>
> I can imagine similar requirements regarding logging, telemetry and dump functions being imposed on select libs.
>
> Perhaps we should also require that new libs implement all four types of instrumentation, to ensure that only high quality (i.e. fully instrumented) libs are accepted into DPDK.

IMO, There is a lot of effort to put trace points to existing
libraries, without these check, soon the disparity shows up when
someone adds new APIs to library and forget
to add trace points.

It is pretty easy to add trace point and there are a lot of
references, so I don't think there is burden  for a developer
comparing to the effort of adding a new API.

Most of the time, contributors forgets to add trace point because
there is no automatic way to find it.
Also, additional git commits are needs if some decide to add it later.

No strong opinion, If not find not useful, we could mark this patch is
not appliable. Keeping the patch is limbo state is the issue. It is
already reached to v5.
So please decide one way or another.




> </irony>
>
> > I don't really like adding a skip list as one more burden for future
> > authors.
>
> If the warning omitted from checkpatches refers to the skip list and its location, it is relatively easy for developers to manage.
>
> And reviewers will notice if new functions have been added to the skip list, indicating that trace has been omitted. So there are also advantages to the skip list.
>