Re: [PATCH 1/2] perf: Add PERF_EVENT_IOC_INC_EVENT_LIMIT

[Charlie Jenkins <charlie@rivosinc.com>]

On Wed, Jul 24, 2024 at 03:43:25PM -0700, Charlie Jenkins wrote:
> PERF_EVENT_IOC_REFRESH immediately enables after incrementing
> event_limit.  Provide a new ioctl flag that allows programs to increment
> event_limit without enabling the event. A usecase for this is to set an
> event_limit in combination with enable_on_exec.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
>  include/linux/perf_event.h            |  4 ++--
>  include/uapi/linux/perf_event.h       |  1 +
>  kernel/events/core.c                  | 17 +++++++++++------
>  tools/include/uapi/linux/perf_event.h |  1 +
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a5304ae8c654..40025a5eb98a 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1103,7 +1103,7 @@ extern int perf_event_task_enable(void);
>  
>  extern void perf_pmu_resched(struct pmu *pmu);
>  
> -extern int perf_event_refresh(struct perf_event *event, int refresh);
> +extern int perf_event_refresh(struct perf_event *event, int refresh, bool enable);
>  extern void perf_event_update_userpage(struct perf_event *event);
>  extern int perf_event_release_kernel(struct perf_event *event);
>  extern struct perf_event *
> @@ -1770,7 +1770,7 @@ static inline int perf_event_read_local(struct perf_event *event, u64 *value,
>  static inline void perf_event_print_debug(void)				{ }
>  static inline int perf_event_task_disable(void)				{ return -EINVAL; }
>  static inline int perf_event_task_enable(void)				{ return -EINVAL; }
> -static inline int perf_event_refresh(struct perf_event *event, int refresh)
> +static inline int perf_event_refresh(struct perf_event *event, int refresh, bool enable)
>  {
>  	return -EINVAL;
>  }
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 3a64499b0f5d..992f51effb27 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -560,6 +560,7 @@ struct perf_event_query_bpf {
>  #define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
>  #define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
>  #define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
> +#define PERF_EVENT_IOC_INC_EVENT_LIMIT		_IO ('$', 12)
>  
>  enum perf_event_ioc_flags {
>  	PERF_IOC_FLAG_GROUP		= 1U << 0,
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 8f908f077935..b9d009733ace 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3142,7 +3142,7 @@ void perf_event_addr_filters_sync(struct perf_event *event)
>  }
>  EXPORT_SYMBOL_GPL(perf_event_addr_filters_sync);
>  
> -static int _perf_event_refresh(struct perf_event *event, int refresh)
> +static int _perf_event_refresh(struct perf_event *event, int refresh, bool enable)
>  {
>  	/*
>  	 * not supported on inherited events
> @@ -3151,7 +3151,8 @@ static int _perf_event_refresh(struct perf_event *event, int refresh)
>  		return -EINVAL;
>  
>  	atomic_add(refresh, &event->event_limit);
> -	_perf_event_enable(event);
> +	if (enable)
> +		_perf_event_enable(event);
>  
>  	return 0;
>  }
> @@ -3159,13 +3160,13 @@ static int _perf_event_refresh(struct perf_event *event, int refresh)
>  /*
>   * See perf_event_disable()
>   */
> -int perf_event_refresh(struct perf_event *event, int refresh)
> +int perf_event_refresh(struct perf_event *event, int refresh, bool enable)
>  {
>  	struct perf_event_context *ctx;
>  	int ret;
>  
>  	ctx = perf_event_ctx_lock(event);
> -	ret = _perf_event_refresh(event, refresh);
> +	ret = _perf_event_refresh(event, refresh, enable);
>  	perf_event_ctx_unlock(event, ctx);
>  
>  	return ret;
> @@ -5920,7 +5921,7 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
>  		break;
>  
>  	case PERF_EVENT_IOC_REFRESH:
> -		return _perf_event_refresh(event, arg);
> +		return _perf_event_refresh(event, arg, true);
>  
>  	case PERF_EVENT_IOC_PERIOD:
>  	{
> @@ -6006,6 +6007,10 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
>  
>  		return perf_event_modify_attr(event,  &new_attr);
>  	}
> +
> +	case PERF_EVENT_IOC_INC_EVENT_LIMIT:
> +		return _perf_event_refresh(event, arg, false);
> +
>  	default:
>  		return -ENOTTY;
>  	}
> @@ -6721,7 +6726,7 @@ void perf_event_wakeup(struct perf_event *event)
>  	ring_buffer_wakeup(event);
>  
>  	if (event->pending_kill) {
> -		kill_fasync(perf_event_fasync(event), SIGIO, event->pending_kill);
> +		kill_fasync(perf_event_fasync(event), SIGTRAP, event->pending_kill);

This was a debug change that wasn't supposed to make it into this patch.
In the next version I will remove this.

- Charlie

>  		event->pending_kill = 0;
>  	}
>  }
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index 3a64499b0f5d..992f51effb27 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -560,6 +560,7 @@ struct perf_event_query_bpf {
>  #define PERF_EVENT_IOC_PAUSE_OUTPUT		_IOW('$', 9, __u32)
>  #define PERF_EVENT_IOC_QUERY_BPF		_IOWR('$', 10, struct perf_event_query_bpf *)
>  #define PERF_EVENT_IOC_MODIFY_ATTRIBUTES	_IOW('$', 11, struct perf_event_attr *)
> +#define PERF_EVENT_IOC_INC_EVENT_LIMIT		_IO ('$', 12)
>  
>  enum perf_event_ioc_flags {
>  	PERF_IOC_FLAG_GROUP		= 1U << 0,
> 
> -- 
> 2.44.0
>