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, URIBL_BLOCKED,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 CEF0EC433ED for ; Tue, 11 May 2021 12:50:33 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 2835860720 for ; Tue, 11 May 2021 12:50:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2835860720 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=desiato.20200630; 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=PQW+TkZn+CzN4esn89u6MI2aNAIYH60iqKX4hb+8qk8=; b=muvjijlKEl659KrbOzzU8F3Qg +akr1v1wyX7wNdWAwWP1aInuXTuWqYmL1tKZ1KBr8GgaCZw5aMVDgrt4qAH88svMonFFtceadSkRq Bsn/nCSXgAlgwArWt5xVxtObWr3MrJctdz9AuZG/Xr5a/et7HKSAp4GXlSZRBwSFMvkZO2Y4aMm+7 8WM5+JO9CnvXOgd0VlkoCG9VpjLy4PzSkKcgpL+bhkxWuK3Pvh6U1yHh51HNBifVanzDl8HYNgiUV F+RWGAWTWau5/HwgRm3z97JVlZLU9D4STHUnQEwQp1Tnlkac8t2LhNVSEiAJhDE/JXFpalLz89J+d DGS1zoUaA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lgRoJ-00HTZm-2g; Tue, 11 May 2021 12:48:28 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgPIw-00Gq7k-Nk for linux-arm-kernel@desiato.infradead.org; Tue, 11 May 2021 10:07:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=FDxcmocipixHZ1RuT7VcSmASKyJ+aD+XLDopZGN3zIo=; b=lhEIc7WaZ49NyI8TdyDU7VrwaC lXgPs0KMpR1KOCP8N6WXgtoPpGSZg39qz42/p1IAlft9LpwoFbiT5r2piJDMaKO42zOkDE4nzB5PB O2PtsPVqZiorifRaG2GUN8GhNDN0//QoZh+NH3PC41UOgMgrUTmvjnJQzkF5gKkJah1WlBoW+bPPN xAsu9FQ8LmNkIknydN9xnheOOHXT7Bj+o5O4sMEpC0r9lrL2mH41vHL9IlOgxogbmEJCPhCoVzZmo mO280sSYKIhvZm7CcCtobYejLLSCphmmc01d9fgmB8AlwjG8fFYDSJSnuuFovJLlMWSfwY+SfQCqU ciTw4gxA==; Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lgPIt-009Sa0-Hi for linux-arm-kernel@lists.infradead.org; Tue, 11 May 2021 10:07:53 +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 A79FF169E; Tue, 11 May 2021 03:07:50 -0700 (PDT) Received: from [10.57.81.122] (unknown [10.57.81.122]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6FDA53F719; Tue, 11 May 2021 03:07:47 -0700 (PDT) Subject: Re: [RFC PATCH] perf cs-etm: Handle valid-but-zero timestamps To: Mike Leach Cc: Coresight ML , Mathieu Poirier , Al Grant , branislav.rankov@arm.com, Denis Nikitin , "Suzuki K. Poulose" , Anshuman Khandual , Leo Yan , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , John Garry , Will Deacon , linux-arm-kernel , linux-perf-users@vger.kernel.org, Linux Kernel Mailing List References: <20210507095814.17933-1-james.clark@arm.com> <3926c523-3fdb-66de-8b9c-b68290a5053e@arm.com> From: James Clark Message-ID: Date: Tue, 11 May 2021 13:07:45 +0300 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: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210511_030751_722026_27EE12E5 X-CRM114-Status: GOOD ( 36.26 ) 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 07/05/2021 17:09, Mike Leach wrote: > Hi James, > > 1) Not sure about this particular target - but previously all 0 > timestamps have been the result of not enabling a clock or the > coresight timestamp generator. > > 2) Given that you can synthesise timestamps when all the values are > all 0 - does this not imply that they could be synthesised when they > are not present at all? Yes they could, perhaps we could change our command line arguments to remove recording of timestamps completely. But I think that the auto enabling of timestamps for per-cpu mode is only additive so there is no way to disable it on the command line. Perhaps we could make a change there instead to make the event configuration more flexible. There is a similar concept with the --timestamp argument where it has both "not set" and "set, but set to false" variables to distinguish between when the user hasn't provided anything and when they've provided the negative: OPT_BOOLEAN_SET('T', "timestamp", &record.opts.sample_time, &record.opts.sample_time_set, opts.sample_time is whether timestamps are on or not and opts.sample_time_set is whether the user provided any argument, true or false. Thanks James > > Cheers > > Mike > > On Fri, 7 May 2021 at 11:02, James Clark wrote: >> >> >> >> On 07/05/2021 12:58, James Clark wrote: >>> There is an intermittent issue on Trogdor devices that >>> results in all Coresight timestamps having a value of zero. >> >> I've attached a file here that has the issue. From the dump you >> can see the zero timestamps: >> >> Idx:69; ID:10; I_TIMESTAMP : Timestamp.; Updated val = 0x0 >> Idx:71; ID:10; I_ATOM_F1 : Atom format 1.; E >> Idx:72; ID:10; I_ADDR_S_IS0 : Address, Short, IS0.; Addr=0xFFFFFFE723C65824 ~[0x5824] >> >> This doesn't have an impact on decoding as they end up being >> decoded in file order as in with timeless mode. >> >> James >> >>> Because zero is currently treated as "not found" rather >>> than "found but zero", this breaks the decoding flow >>> when it would otherwise work. >>> >>> This patch adds an out parameter and return code so >>> the difference between zero and non-existent timestamps >>> can be determined. >>> >>> There is also a change to fix an underflow. >>> >>> Although this is a workaround, the change is technically >>> a valid way of writing the cs_etm__etmq_get_timestamp() >>> function. It could have been written similarly to this >>> without trying to work around this issue, so it's no less >>> correct. But, because it's a workaround to a problem >>> elsewhere, I will submit this as an RFC for feedback. >>> >>> This patch applies on top of the "[PATCH v2 0/2] perf >>> cs-etm: Set time on synthesised samples to preserve ordering" >>> patchset. >>> >>> Co-developed-by: Denis Nikitin >>> Signed-off-by: Denis Nikitin >>> Signed-off-by: James Clark >>> --- >>> .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 5 ++++- >>> tools/perf/util/cs-etm.c | 22 +++++++++---------- >>> 2 files changed, 15 insertions(+), 12 deletions(-) >>> >>> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> index b01d363b9301..947e44413c6e 100644 >>> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> @@ -320,7 +320,10 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq, >>> * which instructions started by subtracting the number of instructions >>> * executed to the timestamp. >>> */ >>> - packet_queue->cs_timestamp = elem->timestamp - packet_queue->instr_count; >>> + if (packet_queue->instr_count >= elem->timestamp) >>> + packet_queue->cs_timestamp = 0; >>> + else >>> + packet_queue->cs_timestamp = elem->timestamp - packet_queue->instr_count; >>> packet_queue->next_cs_timestamp = elem->timestamp; >>> packet_queue->instr_count = 0; >>> >>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c >>> index e5c1a1b22a2a..1969921c406a 100644 >>> --- a/tools/perf/util/cs-etm.c >>> +++ b/tools/perf/util/cs-etm.c >>> @@ -210,13 +210,14 @@ void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq, >>> etmq->pending_timestamp_chan_id = trace_chan_id; >>> } >>> >>> -static u64 cs_etm__etmq_get_timestamp(struct cs_etm_queue *etmq, >>> +static int cs_etm__etmq_get_timestamp(struct cs_etm_queue *etmq, >>> + u64 *timestamp, >>> u8 *trace_chan_id) >>> { >>> struct cs_etm_packet_queue *packet_queue; >>> >>> if (!etmq->pending_timestamp_chan_id) >>> - return 0; >>> + return -ENODATA; >>> >>> if (trace_chan_id) >>> *trace_chan_id = etmq->pending_timestamp_chan_id; >>> @@ -224,13 +225,15 @@ static u64 cs_etm__etmq_get_timestamp(struct cs_etm_queue *etmq, >>> packet_queue = cs_etm__etmq_get_packet_queue(etmq, >>> etmq->pending_timestamp_chan_id); >>> if (!packet_queue) >>> - return 0; >>> + return -ENODATA; >>> >>> /* Acknowledge pending status */ >>> etmq->pending_timestamp_chan_id = 0; >>> >>> /* See function cs_etm_decoder__do_{hard|soft}_timestamp() */ >>> - return packet_queue->cs_timestamp; >>> + if (timestamp) >>> + *timestamp = packet_queue->cs_timestamp; >>> + return 0; >>> } >>> >>> static void cs_etm__clear_packet_queue(struct cs_etm_packet_queue *queue) >>> @@ -864,11 +867,10 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, >>> * Function cs_etm_decoder__do_{hard|soft}_timestamp() does all >>> * the timestamp calculation for us. >>> */ >>> - cs_timestamp = cs_etm__etmq_get_timestamp(etmq, &trace_chan_id); >>> - >>> - /* We found a timestamp, no need to continue. */ >>> - if (cs_timestamp) >>> + if (!cs_etm__etmq_get_timestamp(etmq, &cs_timestamp, &trace_chan_id)) { >>> + /* We found a timestamp, no need to continue. */ >>> break; >>> + } >>> >>> /* >>> * We didn't find a timestamp so empty all the traceid packet >>> @@ -2286,9 +2288,7 @@ static int cs_etm__process_queues(struct cs_etm_auxtrace *etm) >>> if (ret) >>> goto out; >>> >>> - cs_timestamp = cs_etm__etmq_get_timestamp(etmq, &trace_chan_id); >>> - >>> - if (!cs_timestamp) { >>> + if (cs_etm__etmq_get_timestamp(etmq, &cs_timestamp, &trace_chan_id)) { >>> /* >>> * Function cs_etm__decode_data_block() returns when >>> * there is no more traces to decode in the current >>> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel