On Thu, Aug 31, 2023 at 12:08 AM Morten Brørup <m...@smartsharesystems.com> wrote: > > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > Sent: Wednesday, 30 August 2023 18.24 > > > > 21/08/2023 16:46, Morten Brørup: > > > > From: Ankur Dwivedi [mailto:adwiv...@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. >
