Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752223AbdHDK7c (ORCPT ); Fri, 4 Aug 2017 06:59:32 -0400 Received: from merlin.infradead.org ([205.233.59.134]:48776 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751965AbdHDK7a (ORCPT ); Fri, 4 Aug 2017 06:59:30 -0400 Date: Fri, 4 Aug 2017 12:59:22 +0200 From: Peter Zijlstra To: "Naveen N. Rao" Cc: Jiri Olsa , Arnaldo Carvalho de Melo , Ingo Molnar , Vince Weaver , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] kernel/events: Add option to enable counting sideband events in wakeup_events Message-ID: <20170804105922.uaxuj3ote263ofec@hirez.programming.kicks-ass.net> References: <3e7dabc8a4762aad3836244661f843dda689588b.1501576497.git.naveen.n.rao@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3e7dabc8a4762aad3836244661f843dda689588b.1501576497.git.naveen.n.rao@linux.vnet.ibm.com> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1486 Lines: 45 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.