From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E402C07E96 for ; Thu, 15 Jul 2021 03:55:40 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 62D9F61279 for ; Thu, 15 Jul 2021 03:55:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 62D9F61279 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=OQXoURkabYmD6mBuRZg8wixRrD6d/v3hi/tkSLyeXoM=; b=VniFeiTFtOgfzMCOPzN3/WkSCU eEOjf8ooeXjy2YCSMC3gx74jKLULbi+VIjltz+R+5h9b58AhHcx3mmhP3gMGi7uaP6zhoyPe/8eE8 I99nhgnjiiTNOwPJtWLOPkZ/huZTQMs0omGomUBnwDPvaYZ7hQFNsOLRqu59XSNk1d8ru8zB7TOBr wkTcMxgp67nUdYeh4izBbS0Pvel46SvKO09rBZeHC0JyIMOrvN4zhS680oVc/gHQnW0uYLTIRzs6T rWGSXCiFtrob/4Jz+dnqzyR2HIZir0itfvXpkS7qfQNZWGv0+eo09pBdMXeTV36bB0tlCcsJSx1wp wncT4RfA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m3sRE-00H2nj-6r; Thu, 15 Jul 2021 03:53:28 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m3sRA-00H2mB-3i for linux-arm-kernel@lists.infradead.org; Thu, 15 Jul 2021 03:53:25 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7FEA2D6E; Wed, 14 Jul 2021 20:53:21 -0700 (PDT) Received: from [10.163.66.71] (unknown [10.163.66.71]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C44BC3F774; Wed, 14 Jul 2021 20:53:17 -0700 (PDT) Subject: Re: [PATCH 3/5] coresight: trbe: Keep TRBE disabled on overflow irq To: Suzuki K Poulose , linux-arm-kernel@lists.infradead.org Cc: coresight@lists.linaro.org, linux-kernel@vger.kernel.org, al.grant@arm.com, leo.yan@linaro.org, mathieu.poirier@linaro.org, mike.leach@linaro.org, peterz@infradead.org, Tamas.Zsoldos@arm.com, will@kernel.org References: <20210712113830.2803257-1-suzuki.poulose@arm.com> <20210712113830.2803257-4-suzuki.poulose@arm.com> From: Anshuman Khandual Message-ID: <4ea6e548-db54-a3cc-751e-42700d76704f@arm.com> Date: Thu, 15 Jul 2021 09:24:05 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210712113830.2803257-4-suzuki.poulose@arm.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210714_205324_340866_750FB7B1 X-CRM114-Status: GOOD ( 42.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 7/12/21 5:08 PM, Suzuki K Poulose wrote: > When an AUX buffer is marked TRUNCATED, the kernel will disable > the event (via irq_work) and can only be re-enabled by the > userspace tool. Will it be renabled via normal perf event enable callback OR an explicit perf event restart is required upon the TRUNCATED flag ? > > Also, since we *always* mark the buffer as TRUNCATED (which is > needs to be reconsidered, see below), we need not re-enable the > TRBE as the event is going to be now disabled. This follows the > SPE driver behavior. > > Without this change, we could leave the event disabled for > ever under certain conditions. i.e, when we try to re-enable > in the IRQ handler, if there is no space left in the buffer, > we "TRUNCATE" the buffer and create a record of size 0. > The perf tool skips such records and the event will remain > disabled forever. Why ? Should not the user space tool explicitly start back the tracing event after detecting zero sized record with buffer marked with TRUNCATE flag ? What prevents it to restart the event ? > > Regarding the use of TRUNCATED flag: > With FILL mode, the TRBE stops collection when the buffer is > full, raising the IRQ. Now, since the IRQ is asynchronous, > we could loose some amount of trace, after the buffer was > full until the IRQ is handled. Also the last packet may > have been trimmed. The decoder can figure this out and > cope with this. The side effect of this approach is: > > We always disable the event when there is an IRQ, even > when the other half of the ring buffer is free ! This > is not ideal. > > Now, we should switch to using PARTIAL to indicate that there > was potentially some partial trace packets in the buffer and > some data was lost. We should use TRUNCATED only when there > is absolutely no space in the ring buffer. This change would > also require some additional changes in the CoreSight PMU > framework to allow, sinks to "close" the handle (rather > than the PMU driver closing the handle upon event_stop). > So, until that is sorted, let us keep the TRUNCATED flag > and the rework can be addressed separately. But I guess this is a separate problem all together. > > Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") > Reported-by: Tamas Zsoldos > Cc: Anshuman Khandual > Cc: Mathieu Poirier > Cc: Mike Leach > Cc: Leo Yan > Cc: Peter Zijlstra (Intel) > Cc: Will Deacon > Signed-off-by: Suzuki K Poulose > --- > drivers/hwtracing/coresight/coresight-trbe.c | 34 +++++++------------- > 1 file changed, 12 insertions(+), 22 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 176868496879..ec38cf17b693 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -696,10 +696,8 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) > > static void trbe_handle_overflow(struct perf_output_handle *handle) > { > - struct perf_event *event = handle->event; > struct trbe_buf *buf = etm_perf_sink_config(handle); > unsigned long offset, size; > - struct etm_event_data *event_data; > > offset = get_trbe_limit_pointer() - get_trbe_base_pointer(); > size = offset - PERF_IDX2OFF(handle->head, buf); > @@ -709,30 +707,22 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) > /* > * Mark the buffer as truncated, as we have stopped the trace > * collection upon the WRAP event, without stopping the source. > + * > + * We don't re-enable the TRBE here, as the event is > + * bound to be disabled due to the TRUNCATED flag. > + * This is not ideal, as we could use the available space in > + * the ring buffer and continue the tracing. > + * > + * TODO: Revisit the use of TRUNCATED flag and may be instead use > + * PARTIAL, to indicate trace may contain partial packets. > + * And TRUNCATED can be used only if we do not have enough space > + * in the buffer. This would need additional changes in > + * etm_event_stop() to allow the sinks to leave a closed > + * aux_handle. > */ > perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW | > PERF_AUX_FLAG_TRUNCATED); > perf_aux_output_end(handle, size); > - event_data = perf_aux_output_begin(handle, event); > - if (!event_data) { > - /* > - * We are unable to restart the trace collection, > - * thus leave the TRBE disabled. The etm-perf driver > - * is able to detect this with a disconnected handle > - * (handle->event = NULL). > - */ > - trbe_drain_and_disable_local(); > - *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; > - return; > - } > - buf->trbe_limit = compute_trbe_buffer_limit(handle); > - buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf); > - if (buf->trbe_limit == buf->trbe_base) { > - trbe_stop_and_truncate_event(handle); > - return; > - } > - *this_cpu_ptr(buf->cpudata->drvdata->handle) = handle; > - trbe_enable_hw(buf); > } The change here just stops the event restart after handling the IRQ and marking the buffer with PERF_AUX_FLAG_TRUNCATED which helps the event from being disabled for ever (still need to understand how !). I guess it might be better to separate out the problems with using PERF_AUX_FLAG_TRUNCATED and related aspects, in a subsequent patch which just updates the code with the above TODO comment ? > > static bool is_perf_trbe(struct perf_output_handle *handle) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel