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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 Aug 2017 12:59:22 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
Cc:     Jiri Olsa <jolsa@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Vince Weaver <vincent.weaver@...ne.edu>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] kernel/events: Add option to enable counting
 sideband events in wakeup_events

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.