Re: [PATCH V8 01/12] perf/core: Add aux_pause, aux_resume, aux_start_paused

[Adrian Hunter]

On 1/07/24 13:52, Peter Zijlstra wrote:
> On Fri, Jun 28, 2024 at 09:51:00AM +0300, Adrian Hunter wrote:
> 
>> +	union {
>> +		__u32	aux_action;
>> +		struct {
>> +			__u32	aux_start_paused :  1, /* start AUX area tracing paused */
>> +				aux_pause        :  1, /* on overflow, pause AUX area tracing */
>> +				aux_resume       :  1, /* on overflow, resume AUX area tracing */
>> +				__reserved_3     : 29;
>> +		};
>> +	};
> 
>> @@ -12860,7 +12930,7 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,
>>  	 * Grouping is not supported for kernel events, neither is 'AUX',
>>  	 * make sure the caller's intentions are adjusted.
>>  	 */
>> -	if (attr->aux_output)
>> +	if (attr->aux_output || attr->aux_action)
>>  		return ERR_PTR(-EINVAL);
>>  
> 
> AFAICT this is the only usage of aux_action. But in a few patches time
> you'll introduce a helper along the lines of has_aux_action() that tests
> all the individual bits.
> 
> Combined with perf_copy_attr() ensuring __reserved_3 is actually 0, I'm
> thinking that should all be enough to render this aux_action field
> surplus to requirement, and we can simplify all this somewhat, no?

It is used in tool's patches and will be used more when additional
fields are added for Intel PT Trigger Tracing.  See the information
in the cover letter about that:

[PATCH V8 00/12] perf/core: Add ability for an event to "pause" or "resume" AUX area tracing
https://lore.kernel.org/lkml/20240628065111.59718-1-adrian.hunter@xxxxxxxxx/

In general, without aux_action there is no way to pass all the
related fields together except by using struct perf_event_attr
itself.