From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754010AbbJTJ44 (ORCPT ); Tue, 20 Oct 2015 05:56:56 -0400 Received: from mga01.intel.com ([192.55.52.88]:41193 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751789AbbJTJ4y (ORCPT ); Tue, 20 Oct 2015 05:56:54 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,706,1437462000"; d="scan'208";a="584441548" From: Alexander Shishkin To: Mathieu Poirier , gregkh@linuxfoundation.org, a.p.zijlstra@chello.nl, acme@kernel.org, mingo@redhat.com, corbet@lwn.net, nicolas.pitre@linaro.org Cc: adrian.hunter@intel.com, zhang.chunyan@linaro.org, mike.leach@arm.com, tor@ti.com, al.grant@arm.com, pawel.moll@arm.com, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, mathieu.poirier@linaro.org Subject: Re: [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API In-Reply-To: <1445192687-24112-21-git-send-email-mathieu.poirier@linaro.org> References: <1445192687-24112-1-git-send-email-mathieu.poirier@linaro.org> <1445192687-24112-21-git-send-email-mathieu.poirier@linaro.org> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Tue, 20 Oct 2015 12:56:47 +0300 Message-ID: <877fmhhnhc.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mathieu Poirier writes: > Implementing perf related APIs to activate and terminate > a trace session. More specifically dealing with the sink > buffer's internal mechanic along with perf's API to start > and stop interactions with the ring buffers. A matter of preference, but I'd say that it would be easier to review this part if you merged all the buffer related patches together. > +static void etb_reset_buffer(struct coresight_device *csdev, > + struct perf_output_handle *handle, > + void *sink_config) > +{ > + struct cs_buffers *buf = sink_config; > + struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + if (buf) { > + /* > + * In snapshot mode ->data_size holds the new address of the > + * ring buffer's head. The size itself is the whole address > + * range since we want the latest information. > + */ > + if (buf->snapshot) > + handle->head = local_xchg(&buf->data_size, > + buf->nr_pages << PAGE_SHIFT); Does it make sense to do this in etb_update_buffer() instead? > + perf_aux_output_end(handle, local_xchg(&buf->data_size, 0), > + local_xchg(&buf->lost, 0)); The corresponding perf_aux_output_begin() is done in etm_event_add(), I'd suggest that you do this in etm_event_del(), unconditionally. Otherwise you're risking ending up with a refcount leak and all sorts of horror. Regards, -- Alex