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=-9.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 09BF7C433B4 for ; Wed, 12 May 2021 01:22:47 +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 84CE061928 for ; Wed, 12 May 2021 01:22:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 84CE061928 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=wfWwxBobruqtY2d3gDq3pKS+KfsqX7loAmkOy8CU0Ig=; b=nYPShnIroDzWyDBgzVwiNEDH/ wcjsWb0Rw1wgsf0hJUcz/9d3T+t46oQTK9InqNYxveNRU/wFOTeHUAHym1u9LuV2wQ5eDm9NFn+2t rnKRZsZ1D7Yk1Qkd11MpXsiJQpI3IwtUFD/nA2mzBVNqiNc/F5mm2eC7mqbQfFTTStYSJNFjhPX/Y WIond4ryw6qAOyInv+Ll/ArY7FyxtQd43tW2RMXNN2XcEqM935JBlpcv+BBROw/UA/65fTXG7Sb0J 2KbJCWDEYLCmRREHxOH9Wukn/LU2bWYv+m7T61oVKJL2Ieeg6zuFQ8UaPD9GXRuV4/303ABE8f9k3 xCTobT+cA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lgdY8-001gFI-TN; Wed, 12 May 2021 01:20:33 +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 1lgdY5-001gF5-GS for linux-arm-kernel@desiato.infradead.org; Wed, 12 May 2021 01:20:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=79mmyyvcgcm5MQgz71z5Pm9TCwtYlH2PZdifJeM9Fa8=; b=BW4pViSs0r2hPPolS6S9cTZt2C 6aAUicdAqSJN1q44ghMZ2YReRMsAQe7coRIzdpR4ksgsWngmQ//a95NOG31D7yiiesseLrmedhkbX rECmApBS3tmLZuEL/l4qLpUjT6Ufk1wUhbivvCHcUnZtr59Q/+aNtPmlbm6HLFq5Mx62rTpJO4tqo R1Gg0Y2t+0d00nHkaEREIIDl+HdUCyikoSCYCFhZZMr8AJ00ZOKMfmD8Kw7VIhFQ6xSoEwRpryEQ8 Btl867IeK77G6c3xKLRK0MNgGvE56mUfFtwhdhuMqyXSV91fG22WvTjzzlANdq93hLH3ussEHFmFV O4E3VTOQ==; Received: from mail-pg1-x52b.google.com ([2607:f8b0:4864:20::52b]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgdY2-00A25w-AZ for linux-arm-kernel@lists.infradead.org; Wed, 12 May 2021 01:20:27 +0000 Received: by mail-pg1-x52b.google.com with SMTP id u25so6438601pgl.9 for ; Tue, 11 May 2021 18:20:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=79mmyyvcgcm5MQgz71z5Pm9TCwtYlH2PZdifJeM9Fa8=; b=SAN11rMlBumG1qxEo4NmJa0HRO2H8tusyjexJOC2ygvp2TJX7//flaz//BM7aNI34O VBxb+1On7YL9KDpL04FabUztLx2OYSL58e9ncagxcEHjFehUpQttshysy+19Es1xfX9u cr50nu7Q7P0GvmYohdxflObi/rhdXS7OlX+Mx9frFOHziTHWgUA5uyd4x5I9aFAQsLLH a5i38drQOH6a+j6YSR9Be1O9e9Fa6DHNLNmU/QK8dICJIvD/0Kzhsft9vQK1T75fBpge i9nQtLAaqoD0tBoitsEbMC5BdWpC9Qb/gMY/Rje2yaCsnUP9M9gr0u6nMkWvloCy76Pn +AcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=79mmyyvcgcm5MQgz71z5Pm9TCwtYlH2PZdifJeM9Fa8=; b=bPJ5EkqEuKY25hH+nVuxLR/QjiLcGyH2k6GFWj/+HXtAPwGr91n+/KQgjUfh1DvL4Y WDSCMdLHFM48sXGRcpEi8ylr479Yc1dfgtGW0zxopHqCJfsVyDXZhO/vw/La8tTLQ+Ot NlxHhHwRCBXXM5qEnL3CDE+pMi4FfWNWZFKZsiY5lS+TXr1SbrmcLrDPSmyPWjoQgDE2 lujxj/yv5TcJHJkvA0JeKIY9dxfIQ9Xj8YBGc8gjOptf/4EGHkaPSL5SuK2k92I/55HY kqZnpu+rJx0E5M6yJlB8M+j+smiwdezmPM0vDahyAGj2g4NGTAnUNLccrpOGe7vT0vfw qaWg== X-Gm-Message-State: AOAM531YRkdBZKg68aKaR14jhrlLhf7KkxAfMlLiem0gvt/qkDmCuUM6 atCW9rFnEbBHnUVlocfb+EreDBGGa8INdTdd X-Google-Smtp-Source: ABdhPJyQiUJ/cl1vIeqZ0Zg48J4pJ5SxGJqZRQGpQ8ghq2el+0yZtCPrNyM8IfpDjJIXUSh5wDeKiQ== X-Received: by 2002:a62:1a4b:0:b029:24e:ba53:aaa4 with SMTP id a72-20020a621a4b0000b029024eba53aaa4mr33902608pfa.63.1620782423521; Tue, 11 May 2021 18:20:23 -0700 (PDT) Received: from leoy-ThinkPad-X240s (ec2-18-167-84-74.ap-east-1.compute.amazonaws.com. [18.167.84.74]) by smtp.gmail.com with ESMTPSA id v18sm15102644pff.90.2021.05.11.18.20.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 May 2021 18:20:23 -0700 (PDT) Date: Wed, 12 May 2021 09:20:02 +0800 From: Leo Yan To: James Clark Cc: coresight@lists.linaro.org, mathieu.poirier@linaro.org, al.grant@arm.com, branislav.rankov@arm.com, denik@chromium.org, suzuki.poulose@arm.com, anshuman.khandual@arm.com, Mike Leach , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , John Garry , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH] perf cs-etm: Handle valid-but-zero timestamps Message-ID: <20210512012002.GB249068@leoy-ThinkPad-X240s> References: <20210507095814.17933-1-james.clark@arm.com> <3926c523-3fdb-66de-8b9c-b68290a5053e@arm.com> <20210510053904.GB4835@leoy-ThinkPad-X240s> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210511_182026_430782_523756E7 X-CRM114-Status: GOOD ( 33.47 ) 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 Tue, May 11, 2021 at 04:53:35PM +0300, James Clark wrote: [...] > Do you have any idea about what to do in the overflow case? A quick thinking is to connect the kernel timestamp and correlate the overflow case for CoreSight's timestamp, but this approach will cause complexity. And considering if the overflow occurs for not only once before the new kernel timestamp is updated, it's hard to handle for this case. So seems to me, printing warning is a better choice. > I think I will submit a > new patchset that makes the new 'Z' timeless --itrace option work, because that also > fixes this issue, without having to do the original workaround change in this RFC. Good finding for these options for zero timestamps! > But I'd also like to fix this overflow because it masks the issue by making non-zero > timestamps appear even though they aren't valid ones. > > I was thinking that printing a warning in the overflow case would work, but then I would > also print a warning for the zero timestamps, and that would make just a single case, > rather than two. Unless we just have slightly different warning text? Printing two different warnings is okay for me, which is more clear for users. > Something like this? Without the zero timestamp issue, the underflow issue probably wouldn't > be encountered. But at least there would be some visibility if it did: > > 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 059bcec3f651..5d8abccd34ab 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -17,6 +17,7 @@ > > #include "cs-etm.h" > #include "cs-etm-decoder.h" > +#include "debug.h" > #include "intlist.h" > > /* use raw logging */ > @@ -294,9 +295,11 @@ cs_etm_decoder__do_soft_timestamp(struct cs_etm_queue *etmq, > static ocsd_datapath_resp_t > cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq, > const ocsd_generic_trace_elem *elem, > - const uint8_t trace_chan_id) > + const uint8_t trace_chan_id, > + const ocsd_trc_index_t indx) Do we really need the new argument "indx"? If print "trace_chan_id", can it give the info that the timestamp is attached to which tracer? > { > struct cs_etm_packet_queue *packet_queue; > + static bool warned_timestamp_zero = false; > > /* First get the packet queue for this traceID */ > packet_queue = cs_etm__etmq_get_packet_queue(etmq, trace_chan_id); > @@ -320,7 +323,20 @@ 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->timestamp = elem->timestamp - packet_queue->instr_count; > + if (!elem->timestamp) { > + packet_queue->timestamp = 0; > + if (!warned_timestamp_zero) { > + pr_err("Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR > + ". Decoding may be improved with --itrace=Z...\n", indx); > + warned_timestamp_zero = true; > + } I think this warning and the next warning for overflow, both can use the macro WARN_ONCE(), so you can avoid to add new variable "warned_timestamp_zero". Thanks, Leo > + } > + else if (packet_queue->instr_count >= elem->timestamp) { > + packet_queue->timestamp = 0; > + pr_err("Timestamp calculation underflow at Idx:%" OCSD_TRC_IDX_STR "\n", indx); > + } > + else > + packet_queue->timestamp = elem->timestamp - packet_queue->instr_count; > packet_queue->next_timestamp = elem->timestamp; > packet_queue->instr_count = 0; > > @@ -542,7 +558,7 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq, > > static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > const void *context, > - const ocsd_trc_index_t indx __maybe_unused, > + const ocsd_trc_index_t indx, > const u8 trace_chan_id __maybe_unused, > const ocsd_generic_trace_elem *elem) > { > @@ -579,7 +595,8 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( > break; > case OCSD_GEN_TRC_ELEM_TIMESTAMP: > resp = cs_etm_decoder__do_hard_timestamp(etmq, elem, > - trace_chan_id); > + trace_chan_id, > + indx); > break; > case OCSD_GEN_TRC_ELEM_PE_CONTEXT: > resp = cs_etm_decoder__set_tid(etmq, packet_queue, > > > James > > > > > Thanks, > > Leo > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel