[FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
Steven Rostedt
rostedt at goodmis.org
Sat Feb 24 05:46:53 AEDT 2024
On Fri, 23 Feb 2024 10:30:45 -0800
Jeff Johnson <quic_jjohnson at quicinc.com> wrote:
> On 2/23/2024 9:56 AM, Steven Rostedt wrote:
> > From: "Steven Rostedt (Google)" <rostedt at goodmis.org>
> >
> > [
> > This is a treewide change. I will likely re-create this patch again in
> > the second week of the merge window of v6.9 and submit it then. Hoping
> > to keep the conflicts that it will cause to a minimum.
> > ]
> >
> > With the rework of how the __string() handles dynamic strings where it
> > saves off the source string in field in the helper structure[1], the
> > assignment of that value to the trace event field is stored in the helper
> > value and does not need to be passed in again.
>
> Just curious if this could be done piecemeal by first changing the
> macros to be variadic macros which allows you to ignore the extra
> argument. The callers could then be modified in their separate trees.
> And then once all the callers have be merged, the macros could be
> changed to no longer be variadic.
I weighed doing that, but I think ripping off the band-aid is a better
approach. One thing I found is that leaving unused parameters in the macros
can cause bugs itself. I found one case doing my clean up, where an unused
parameter in one of the macros was bogus, and when I made it a used
parameter, it broke the build.
I think for tree-wide changes, the preferred approach is to do one big
patch at once. And since this only affects TRACE_EVENT() macros, it
hopefully would not be too much of a burden (although out of tree users may
suffer from this, but do we care?)
Now one thing I could do is to not remove the parameter, but just add:
WARN_ON_ONCE((src) != __data_offsets->item##_ptr_);
in the __assign_str() macro to make sure that it's still the same that is
assigned. But I'm not sure how useful that is, and still causes burden to
have it. I never really liked the passing of the string in two places to
begin with.
-- Steve
More information about the Linuxppc-dev
mailing list