Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 31 Oct 2022 09:38:01 -0700
From:   Beau Belgrave <beaub@linux.microsoft.com>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc:     rostedt@goodmis.org, mhiramat@kernel.org,
        dcook@linux.microsoft.com, alanau@linux.microsoft.com,
        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
Message-ID: <20221031163749.GA69@W11-BEAU-MD.localdomain>
References: <20221027224011.2075-1-beaub@linux.microsoft.com>
 <20221027224011.2075-2-beaub@linux.microsoft.com>
 <7cad3dde-b1c5-935d-23bc-61e98c3a7b07@efficios.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7cad3dde-b1c5-935d-23bc-61e98c3a7b07@efficios.com>
Precedence: bulk

On Sat, Oct 29, 2022 at 10:44:32AM -0400, Mathieu Desnoyers wrote:
> 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
> 

Sure, I'll change this __u8.

> 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