Re: [PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Aug 01, 2017 at 08:14:04PM +0530, Naveen N. Rao wrote:
> @@ -5974,19 +5976,8 @@ void perf_output_sample(struct perf_output_handle *handle,
>  		}
>  	}
>  
> +	if (!event->attr.count_sb_events)
> +		rb_handle_wakeup_events(event, handle->rb);
>  }

> +void __always_inline
> +rb_handle_wakeup_events(struct perf_event *event, struct ring_buffer *rb)
> +{
> +	int wakeup_events = event->attr.wakeup_events;
> +
> +	if (!event->attr.watermark && wakeup_events) {
> +		int events = local_inc_return(&rb->events);
> +
> +		if (events >= wakeup_events) {
> +			local_sub(wakeup_events, &rb->events);
> +			local_inc(&rb->wakeup);
> +		}
> +	}
> +}
> +
>  static int __always_inline
>  __perf_output_begin(struct perf_output_handle *handle,
>  		    struct perf_event *event, unsigned int size,
> @@ -197,6 +212,9 @@ __perf_output_begin(struct perf_output_handle *handle,
>  	 * none of the data stores below can be lifted up by the compiler.
>  	 */
>  
> +	if (unlikely(event->attr.count_sb_events))
> +		rb_handle_wakeup_events(event, rb);
> +
>  	if (unlikely(head - local_read(&rb->wakeup) > rb->watermark))
>  		local_add(rb->watermark, &rb->wakeup);
>  

I'm still slightly uneasy over this.. Yes most of our events are
samples, so we'd pay the overhead already. But could you still look at
performance of this, see for example this commit:

  9ecda41acb97 ("perf/core: Add ::write_backward attribute to perf event")

we went through a lot of variants to not hurt performance.