Re: [RFC PATCH 1/2] tracing/user_events: Use remote writes for event enablement

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Beau Belgrave <beaub@linux.microsoft.com>,
	rostedt@goodmis.org, mhiramat@kernel.org,
	dcook@linux.microsoft.com, alanau@linux.microsoft.com
Cc: linux-trace-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/2] tracing/user_events: Use remote writes for event enablement
Date: Sat, 29 Oct 2022 10:44:32 -0400	[thread overview]
Message-ID: <7cad3dde-b1c5-935d-23bc-61e98c3a7b07@efficios.com> (raw)
In-Reply-To: <20221027224011.2075-2-beaub@linux.microsoft.com>

On 2022-10-27 18:40, Beau Belgrave wrote:

[...]
> diff --git a/include/linux/user_events.h b/include/linux/user_events.h
> index 592a3fbed98e..4c3bd16395a9 100644
> --- a/include/linux/user_events.h
> +++ b/include/linux/user_events.h
> @@ -33,12 +33,16 @@ struct user_reg {
>   	/* Input: Size of the user_reg structure being used */
>   	__u32 size;
>   
> +	/* Input: Flags/common settings */
> +	__u32 enable_bit : 5, /* Bit in enable address to use (0-31) */
> +	      __reserved : 27;

I'm always worried about using C/C++ bitfields in uapi, because some 
compilers (e.g. MS Windows Compiler) have different implementation of 
the bitfields. See

gcc(1)

-Wnopacked-bitfield-compat
-mms-bitfields / -mno-ms-bitfields

Thanks,

Mathieu

> +
> +	/* Input: Address to update when enabled */
> +	__u64 enable_addr;
> +
>   	/* Input: Pointer to string with event name, description and flags */
>   	__u64 name_args;
>   
> -	/* Output: Bitwise index of the event within the status page */
> -	__u32 status_bit;
> -
>   	/* Output: Index of the event to use when writing data */
>   	__u32 write_index;
>   } __attribute__((__packed__));
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com