2024-02-26 19:17:31

by Chuck Lever

[permalink] [raw]
Subject: Re: [for-next][PATCH 2/5] tracing: Remove __assign_str_len()

On Mon, Feb 26, 2024 at 01:49:34PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <[email protected]>
>
> Now that __assign_str() gets the length from the __string() (and
> __string_len()) macros, there's no reason to have a separate
> __assign_str_len() macro as __assign_str() can get the length of the
> string needed.
>
> Also remove __assign_rel_str() although it had no users anyway.
>
> Link: https://lore.kernel.org/linux-trace-kernel/[email protected]
>
> Cc: Chuck Lever <[email protected]>
> Cc: Jeff Layton <[email protected]>
> Signed-off-by: Steven Rostedt (Google) <[email protected]>

Acked-by: Chuck Lever <[email protected]>


> ---
> fs/nfsd/trace.h | 8 +++---
> include/trace/stages/stage6_event_callback.h | 28 ++++++++------------
> samples/trace_events/trace-events-sample.h | 9 ++++---
> 3 files changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 2cd57033791f..98b14f30d772 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -102,7 +102,7 @@ TRACE_EVENT(nfsd_compound,
> TP_fast_assign(
> __entry->xid = be32_to_cpu(rqst->rq_xid);
> __entry->opcnt = opcnt;
> - __assign_str_len(tag, tag, taglen);
> + __assign_str(tag, tag);
> ),
> TP_printk("xid=0x%08x opcnt=%u tag=%s",
> __entry->xid, __entry->opcnt, __get_str(tag)
> @@ -483,7 +483,7 @@ TRACE_EVENT(nfsd_dirent,
> TP_fast_assign(
> __entry->fh_hash = fhp ? knfsd_fh_hash(&fhp->fh_handle) : 0;
> __entry->ino = ino;
> - __assign_str_len(name, name, namlen)
> + __assign_str(name, name);
> ),
> TP_printk("fh_hash=0x%08x ino=%llu name=%s",
> __entry->fh_hash, __entry->ino, __get_str(name)
> @@ -853,7 +853,7 @@ DECLARE_EVENT_CLASS(nfsd_clid_class,
> __entry->flavor = clp->cl_cred.cr_flavor;
> memcpy(__entry->verifier, (void *)&clp->cl_verifier,
> NFS4_VERIFIER_SIZE);
> - __assign_str_len(name, clp->cl_name.data, clp->cl_name.len);
> + __assign_str(name, clp->cl_name.data);
> ),
> TP_printk("addr=%pISpc name='%s' verifier=0x%s flavor=%s client=%08x:%08x",
> __entry->addr, __get_str(name),
> @@ -1800,7 +1800,7 @@ TRACE_EVENT(nfsd_ctl_time,
> TP_fast_assign(
> __entry->netns_ino = net->ns.inum;
> __entry->time = time;
> - __assign_str_len(name, name, namelen);
> + __assign_str(name, name);
> ),
> TP_printk("file=%s time=%d\n",
> __get_str(name), __entry->time
> diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
> index 2bfd49713b42..0c0f50bcdc56 100644
> --- a/include/trace/stages/stage6_event_callback.h
> +++ b/include/trace/stages/stage6_event_callback.h
> @@ -32,16 +32,13 @@
>
> #undef __assign_str
> #define __assign_str(dst, src) \
> - memcpy(__get_str(dst), __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, \
> - __get_dynamic_array_len(dst))
> -
> -#undef __assign_str_len
> -#define __assign_str_len(dst, src, len) \
> do { \
> - memcpy(__get_str(dst), \
> - __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
> - __get_str(dst)[len] = '\0'; \
> - } while(0)
> + char *__str__ = __get_str(dst); \
> + int __len__ = __get_dynamic_array_len(dst) - 1; \
> + memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
> + EVENT_NULL_STR, __len__); \
> + __str__[__len__] = '\0'; \
> + } while (0)
>
> #undef __assign_vstr
> #define __assign_vstr(dst, fmt, va) \
> @@ -94,15 +91,12 @@
>
> #undef __assign_rel_str
> #define __assign_rel_str(dst, src) \
> - memcpy(__get_rel_str(dst), __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, \
> - __get_rel_dynamic_array_len(dst))
> -
> -#undef __assign_rel_str_len
> -#define __assign_rel_str_len(dst, src, len) \
> do { \
> - memcpy(__get_rel_str(dst), \
> - __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
> - __get_rel_str(dst)[len] = '\0'; \
> + char *__str__ = __get_rel_str(dst); \
> + int __len__ = __get_rel_dynamic_array_len(dst) - 1; \
> + memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
> + EVENT_NULL_STR, __len__); \
> + __str__[__len__] = '\0'; \
> } while (0)
>
> #undef __rel_bitmask
> diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
> index 23f923ccd529..f2d2d56ce8e2 100644
> --- a/samples/trace_events/trace-events-sample.h
> +++ b/samples/trace_events/trace-events-sample.h
> @@ -163,8 +163,7 @@
> * __string().
> *
> * __string_len: This is a helper to a __dynamic_array, but it understands
> - * that the array has characters in it, and with the combined
> - * use of __assign_str_len(), it will allocate 'len' + 1 bytes
> + * that the array has characters in it, it will allocate 'len' + 1 bytes
> * in the ring buffer and add a '\0' to the string. This is
> * useful if the string being saved has no terminating '\0' byte.
> * It requires that the length of the string is known as it acts
> @@ -174,9 +173,11 @@
> *
> * __string_len(foo, bar, len)
> *
> - * To assign this string, use the helper macro __assign_str_len().
> + * To assign this string, use the helper macro __assign_str().
> + * The length is saved via the __string_len() and is retrieved in
> + * __assign_str().
> *
> - * __assign_str_len(foo, bar, len);
> + * __assign_str(foo, bar);
> *
> * Then len + 1 is allocated to the ring buffer, and a nul terminating
> * byte is added. This is similar to:
> --
> 2.43.0
>
>

--
Chuck Lever