Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-02-23 18:45:06
Also in:
amd-gfx, ath11k, ath12k, bpf, dri-devel, intel-gfx, intel-xe, io-uring, kvm, linux-arm-msm, linux-bcachefs, linux-block, linux-btrfs, linux-cifs, linux-cxl, linux-edac, linux-f2fs-devel, linux-hwmon, linux-hyperv, linux-iommu, linux-media, linux-nfs, linux-pm, linux-rdma, linux-s390, linux-sound, linux-tegra, linux-trace-kernel, linux-usb, linux-wireless, linux-xfs, lkml, netdev, ocfs2-devel, selinux, virtualization
On Fri, 23 Feb 2024 10:30:45 -0800
Jeff Johnson [off-list ref] wrote:
On 2/23/2024 9:56 AM, Steven Rostedt wrote:
quoted
From: "Steven Rostedt (Google)" <rostedt@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