21/08/2023 16:46, Morten Brørup: > > From: Ankur Dwivedi [mailto:adwiv...@marvell.com] > > Sent: Monday, 21 August 2023 15.54 > > > > >From: Stephen Hemminger <step...@networkplumber.org> > > >Sent: Thursday, May 18, 2023 9:04 PM > > > > > >---------------------------------------------------------------------- > > >On Thu, 18 May 2023 13:45:29 +0000 > > >Ankur Dwivedi <adwiv...@marvell.com> wrote: > > > > > >> >From: Ankur Dwivedi <adwiv...@marvell.com> > > >> >Sent: Tuesday, March 7, 2023 5:35 PM > > >> > > > >> >This patch adds a validation in checkpatch tool, to check if a > > >> >tracepoint is present in any new function added in cryptodev, > > ethdev, > > >> >eventdev and mempool library. > > >> > > > >> >In this patch, the build_map_changes function is copied from > > >> >check-symbol- change.sh to check-tracepoint.sh. The > > >> >check-tracepoint.sh script uses build_map_changes function to create > > a > > >map of functions. > > >> >In the map, the newly added functions, added in the experimental > > >> >section are identified and their definition are checked for the > > >> >presence of tracepoint. The checkpatch return error if the > > tracepoint is not > > >present. > > >> > > > >> >For functions for which trace is not needed, they can be added to > > >> >devtools/trace-skiplist.txt file. The above tracepoint check will be > > >> >skipped for them. > > >> > > > >> >Signed-off-by: Ankur Dwivedi <adwiv...@marvell.com> > > > > > >Given the amount of string processing, it would be more readable in > > python. > > >That is not a show stopper, just a suggestion. > > > > Hi Thomas, > > > > Please let me know if the shell script in this patch is fine or would a > > python implementation would be more preferable.
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 don't really like adding a skip list as one more burden for future authors. > Nonetheless, having a tool to check for tracepoint presence might still be > useful for library reviewers and maintainers. And such a tool might be useful > for any library, not just the few libraries suggested by this patch.
