Received-SPF: pass (google.com: domain of linux-kernel+bounces-82223-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 26 Feb 2024 14:15:59 -0500
From: Chuck Lever <chuck.lever@oracle.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Jeff Layton <jlayton@kernel.org>
Subject: Re: [for-next][PATCH 2/5] tracing: Remove __assign_str_len()
Message-ID: <Zdzjb91cJKvgHUMm@manet.1015granger.net>
References: <20240226184932.303400070@goodmis.org>
 <20240226184950.974920801@goodmis.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240226184950.974920801@goodmis.org>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?rExiuTHuGuCwDXZCuzNwtpjXKy/SSZIBMiTwPL+BHzOiWkI72z0CvHjZRNZZ?=
 =?us-ascii?Q?G/ufcAtMW+BnKe433+7Vkbj0Pp/gfbge+x10FYdkJxqtvzygP3FX8IhVaPJM?=
 =?us-ascii?Q?G3aPMeQAIvsJZ4eUs6Uc8CtG0V5i8B9+1pf7fjyhs769Sy8jPzpTb7hr3KkP?=
 =?us-ascii?Q?zpVNtSyTnyDWLnP4DeLniHzVCamF3XLdi8aAKsYfsHi4AjUfgRWKWLBSPX9b?=
 =?us-ascii?Q?r1LMdjpDI6ZOwslZmqa/xt1X06xyw5B8FicSV9SUiZOs3hKfh3VXYpoZPg+z?=
 =?us-ascii?Q?Wgp+pL7Y90lLO1DBxwtWD482n+/A1WOxHLE3rMpl3aFBOCAK040NjePDibuP?=
 =?us-ascii?Q?N2Ko5hDeqQD+7qdhTeQgtQzJA3aw3FDlj+lXpauhjyfuFm4R5xHvaDvsREeq?=
 =?us-ascii?Q?C055ryvF0CsTv+7QnBIOUx++x0FShVqT26+UnrlE/1IUOWOKm/OHpNK6wbMw?=
 =?us-ascii?Q?qbwy3u3eTLR0fLs75A60LOrNSmdDOAUW/AE0EOEsB/xV4YXBnIhv2WxwwrfA?=
 =?us-ascii?Q?IomRB4tYOBGmuy4KlCenLFIuLqdM2KAWsG6oZ+B9O3+LOWhXgQeV3QX5m7Mi?=
 =?us-ascii?Q?ZoXG1qUSdzrDFGB/A7SQakhAgaO73EF7FuipbOb+jQs+pBvcuZj5qJTM9O30?=
 =?us-ascii?Q?9umIHMOYjeHJR+F5Lct1tU0CEEjqRjg51umXSrP5ZvMBu1bTTokXRjRxcVFY?=
 =?us-ascii?Q?0Tw0g6cm2wkaV72EUwFyKtU3qawB23Xw4V+AnnyZ3l5s9BuokKlfcNnMFeVn?=
 =?us-ascii?Q?fniPRHzWWISC/nxMTuaihPtsG1lBznMhfT6ifLOeTTeSy1nE2iwJzul7vViI?=
 =?us-ascii?Q?nytn+b6Ex8Fj/CnEmEWKlwyrlx3ymmzBjwnhQGg5LjR3sPH0TI86cMbH4iWk?=
 =?us-ascii?Q?8JuG+aAHvvTsiCSrEBaE/cgreZ9UC7R4cDTVxVmTMIBmgeGr2so33d3Ic2zY?=
 =?us-ascii?Q?S2ULbJPFsYRrkWYmk98OIQmXjQuaMshlgGBzSb1HUiRnyyomb+pL776XUBxz?=
 =?us-ascii?Q?Bq62VZgovsxEzUQEixDDVxRfp0ne2wP885LjCByCRz2dht31cLUZ54ropxZN?=
 =?us-ascii?Q?CIEfxfIJ/uamAIdBUFUZxvFd079yZ2JFVXF6R2BQNGemOa0U/DAu7HJj2beR?=
 =?us-ascii?Q?Ng/dImf/hd2Gt9Wa/jLd6c/hXQ5CGTHuPzZDLOZ4i1jd6P6EUCzT/mm6oizz?=
 =?us-ascii?Q?elUHYxXwOSlV7CU9Bu/96qEvSD0hBWJ01KUzb8wVgfaOIq9JwX7XFwJBvGBt?=
 =?us-ascii?Q?LJi5aYtQNKbQSFIJ/GcrgWBm/jiHMLxvtW/FGqfv04kxrD82zSp8HpkYSFhT?=
 =?us-ascii?Q?gsLmsipPxriQ99ySWrT+XXDU8dld97NfftH8TUtBXLW8s4SqMfRcN2DIXUnO?=
 =?us-ascii?Q?yjmxgqAUBrytlLGPWc5zqVur6kFpO0LQUNSQ4ogqlMvXJ1SDt64LAdAAP68M?=
 =?us-ascii?Q?qqGuMMCJS9U9/QiGUgMMNEgAjBLm0M7DM9R1UrvOxNMz4mvsS8MVXqkf+slW?=
 =?us-ascii?Q?6gwJm2Y5gpaPUfDdLcVF5/HJ6dnUmCHKmskZ9NxzlJDkxYQ9Wjc8DCgiUgue?=
 =?us-ascii?Q?50s57xggP308ZY26JP29QoGPn6NGMmOhEH7XiDKBDvZYvWItxhC79nFkmpXy?=
 =?us-ascii?Q?6w=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	PucHzyf9TjikbvaMG2LLURXVGHfBFCHQZLyJNjzMgiQGkD9VmpPNqIHSePCGY6Ss2e7WqWC44Bl3KVOVCDoCCHbL42q7eJJSVHfvkZO0XCt+Q6oRsTWsYZDDxb2/bLq5G9AyVtYOfhOSN2m32YxRpgSiiI0ppSwiP/nKcFhZAXQmP4hTwnG8Mu0X9dapImUUM7LMdX+j0vG2CvYrXWqF2xgDjZDAeIgRqNjBwlSgqy4RI1SPA2NVv6w56U6jJ7ccRSfOWFbfvCpyICet5fXVMRVK5yA5EAcWJLo32X4blXXwWCVfyO+ocqt5ZzrBQeGS41KqJtwN9bjILYuhBdQlwmWDySTMf2j526XNRa0BrN+kJo+CkVu9KinB5NXy5oXaq9TKQ5XmXXtR8T762MlvuQxaUM7yKBp8jPFtJQRjsNXIf8srM4ADodmmdTQQnTLy4D6bYOzwlV7dTwBHCFfYV+99Who/OvFuL0qRllPYu4jpcxi4uWKH8UTxSWjTkGPlho+ZFw9cDwvXNtLOO0wVVXUpUBJGsTdEaS+07boGL9dcc4X2fuiie7QPepG5hDm2rMj6fm9/W6iNMLq48pwt1HNCuB//gc7WSPupRBM+zhs=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b1a764fa-713e-4fbb-bbba-08dc36ff5f80
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Feb 2024 19:16:02.7317
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: y8+pIECzTIwjWJu0qLSkPVqfEstU8pD0NAFbh+sNOt3w/7Yf8D0ZNc+uzIbwJogGnCTbZqURPioR2ujTjiIFiw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB6440
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-26_11,2024-02-26_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 phishscore=0
 adultscore=0 mlxlogscore=999 malwarescore=0 spamscore=0 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402260147
X-Proofpoint-ORIG-GUID: ZvMYk6FJbNtakVLZPfc01y25mzE4S5IC
X-Proofpoint-GUID: ZvMYk6FJbNtakVLZPfc01y25mzE4S5IC

On Mon, Feb 26, 2024 at 01:49:34PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> 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/20240223152206.0b650659@gandalf.local.home
> 
> Cc: Chuck Lever <chuck.lever@oracle.com>
> Cc: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Acked-by: Chuck Lever <chuck.lever@oracle.com>


> ---
>  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