From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752012AbdBUKms (ORCPT ); Tue, 21 Feb 2017 05:42:48 -0500 Received: from foss.arm.com ([217.140.101.70]:58242 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751325AbdBUKml (ORCPT ); Tue, 21 Feb 2017 05:42:41 -0500 Date: Tue, 21 Feb 2017 10:42:43 +0000 From: Will Deacon To: Alexander Shishkin Cc: Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, vince@deater.net, eranian@google.com, Arnaldo Carvalho de Melo , Mathieu Poirier Subject: Re: [PATCH 2/4] perf: Keep AUX flags in the output handle Message-ID: <20170221104242.GB300@arm.com> References: <20170220133352.17995-1-alexander.shishkin@linux.intel.com> <20170220133352.17995-3-alexander.shishkin@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170220133352.17995-3-alexander.shishkin@linux.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Alexander, Thanks for picking this up/adapting it. Just one comment below On Mon, Feb 20, 2017 at 03:33:50PM +0200, Alexander Shishkin wrote: > From: Will Deacon > > In preparation for adding more flags to perf AUX records, introduce a > separate API for setting the flags for a session, rather than appending > more bool arguments to perf_aux_output_end. This allows to set each > flag at the time a corresponding condition is detected, instead of > tracking it in each driver's private state. > > Cc: Mathieu Poirier > Signed-off-by: Will Deacon > Signed-off-by: Alexander Shishkin > --- --->8 > +void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags) > +{ > + /* > + * OVERWRITE is determined by perf_aux_output_end() and can't > + * be passed in directly. > + */ > + if (WARN_ON_ONCE(flags & PERF_AUX_FLAG_OVERWRITE)) > + return; Now that you've added this check... > + handle->aux_flags |= flags; > +} > +EXPORT_SYMBOL_GPL(perf_aux_output_flag); > + > /* > * This is called before hardware starts writing to the AUX area to > * obtain an output handle and make sure there's room in the buffer. > @@ -360,6 +373,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle, > handle->event = event; > handle->head = aux_head; > handle->size = 0; > + handle->aux_flags = 0; > > /* > * In overwrite mode, AUX data stores do not depend on aux_tail, > @@ -409,34 +423,32 @@ EXPORT_SYMBOL_GPL(perf_aux_output_begin); > * of the AUX buffer management code is that after pmu::stop(), the AUX > * transaction must be stopped and therefore drop the AUX reference count. > */ > -void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size, > - bool truncated) > +void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size) > { > struct ring_buffer *rb = handle->rb; > - bool wakeup = truncated; > + bool wakeup = !!handle->aux_flags; > unsigned long aux_head; > - u64 flags = 0; > - > - if (truncated) > - flags |= PERF_AUX_FLAG_TRUNCATED; > > /* in overwrite mode, driver provides aux_head via handle */ > if (rb->aux_overwrite) { > - flags |= PERF_AUX_FLAG_OVERWRITE; > + handle->aux_flags |= PERF_AUX_FLAG_OVERWRITE; > > aux_head = handle->head; > local_set(&rb->aux_head, aux_head); > } else { > + handle->aux_flags &= ~PERF_AUX_FLAG_OVERWRITE; > + ... I don't think you need this addition anymore. It's harmless, though. Will