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=-15.3 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 79ACDC433ED for ; Wed, 14 Apr 2021 15:56:42 +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 09FEC6112F for ; Wed, 14 Apr 2021 15:56:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 09FEC6112F 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-Type: Content-Transfer-Encoding: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=bn0yCyB5orxzNyIGv1sBZMLloOQdRj7td26kha94shE=; b=RoYhDtb5fCrJxwxbAZyAsn3Zl hnH3j0Lq2EOZ1NduWteZ2cWlqN4XKEl1JPThNABvjlGZTZo9qQq/1A2031nLbwazw7wetUB8bJDv+ 3OPZ973UWUKphs7Uy09it+VuPZMxsi/138u6Cavway2G3paxZfT4ibtLyoOIEjjb7bQNBnie5Tz3e 6NxpBvyTYcR21O/FfhfWKjBMlEfjEvnbovJFmw0LJLpLYMr3z2BkbKifi4D6ZeOLQPm2ly7YZv/CP lRv8fc9YvMKfywN5Bc9DO/gMizc83FES4JIGEff5LoORBTmft3b4V33TwYLzLupdzImKamANBwBc9 Ei2VSI66g==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lWhrG-00D4pH-VV; Wed, 14 Apr 2021 15:55:15 +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 1lWhrC-00D4nv-GP for linux-arm-kernel@desiato.infradead.org; Wed, 14 Apr 2021 15:55:10 +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=2uIFNEyogLI5oDqULixgMq7+JQ9Eajl2R8ejOPoveOA=; b=oM0o9QpEyNiNYs3QQKmvwG0lsA VLVsY1CgaLQSLOs6KEjrpWHCHtmDRZL/lM/KAxc60nFSQ85vpOz2JPxyVXm5HZWVXb5JWjD0wL3SX Y6Nk/3bz5gq6l0ziPZfJn/7VduseBUvQEKUDGr7cZB23fQQT9evJzz5zgGgBqwKo7A4kQB52XqsCN e32HDpJuyz9gUtHU5seCeH56oR9/3W++xDGaF0Ix8K2zkOdAVkXSsaTzmUANNzO2FRUbdU1H7qmg4 HGOBXRXdMYBxzvn0w9+M6mQ2IFo8UnPmOb7GrT5Adrj4CFMhJrygCo1fsXbtoMbepu4OyY26z/lfT eiL38Vsw==; Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lWhr9-007vMn-F5 for linux-arm-kernel@lists.infradead.org; Wed, 14 Apr 2021 15:55:09 +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 97E4A11B3; Wed, 14 Apr 2021 08:54:59 -0700 (PDT) Received: from [10.57.27.217] (unknown [10.57.27.217]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 12BFB3F73B; Wed, 14 Apr 2021 08:54:56 -0700 (PDT) Subject: Re: [PATCH 2/2] perf cs-etm: Set time on synthesised samples to preserve ordering To: James Clark , coresight@lists.linaro.org Cc: al.grant@arm.com, branislav.rankov@arm.com, denik@chromium.org, Mike Leach , Leo Yan , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , John Garry , Will Deacon , Mathieu Poirier , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210414143919.12605-1-james.clark@arm.com> <20210414143919.12605-2-james.clark@arm.com> From: Suzuki K Poulose Message-ID: Date: Wed, 14 Apr 2021 16:54:55 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <20210414143919.12605-2-james.clark@arm.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210414_085507_653959_5722F346 X-CRM114-Status: GOOD ( 33.17 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 14/04/2021 15:39, James Clark wrote: > The following attribute is set when synthesising samples in > timed decoding mode: > > attr.sample_type |= PERF_SAMPLE_TIME; > > This results in new samples that appear to have timestamps but > because we don't assign any timestamps to the samples, when the > resulting inject file is opened again, the synthesised samples > will be on the wrong side of the MMAP or COMM events. > > For example this results in the samples being associated with > the perf binary, rather than the target of the record: > > perf record -e cs_etm/@tmc_etr0/u top > perf inject -i perf.data -o perf.inject --itrace=i100il > perf report -i perf.inject > > Where 'Command' == perf should show as 'top': > > # Overhead Command Source Shared Object Source Symbol Target Symbol Basic Block Cycles > # ........ ....... .................... ...................... ...................... .................. > # > 31.08% perf [unknown] [.] 0x000000000040c3f8 [.] 0x000000000040c3e8 - > > If the perf.data file is opened directly with perf, without the > inject step, then this already works correctly because the > events are synthesised after the COMM and MMAP events and > no second sorting happens. Re-sorting only happens when opening > the perf.inject file for the second time so timestamps are > needed. > > Using the timestamp from the AUX record mirrors the current > behaviour when opening directly with perf, because the events > are generated on the call to cs_etm__process_queues(). I would add the following here to clarify the situation : "The ETM trace could optionally contain time stamps, but there is no way to correlate this with the kernel time. So, the best available time value is that of the AUX_RECORD header. This patch uses the timestamp from the header for all the samples. The ordering of the samples are implicit in the trace and thus is fine with respect to relative ordering." > > Signed-off-by: James Clark > Co-developed-by: Al Grant > Signed-off-by: Al Grant nit: The preferred order is your S-o-B at the end (i.e of the sender) > --- > tools/perf/util/cs-etm.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index c25da2ffa8f3..d0fa9dce47f1 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -54,6 +54,7 @@ struct cs_etm_auxtrace { > u8 sample_instructions; > > int num_cpu; > + u64 latest_kernel_timestamp; > u32 auxtrace_type; > u64 branches_sample_type; > u64 branches_id; > @@ -1192,6 +1193,8 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, > event->sample.header.misc = cs_etm__cpu_mode(etmq, addr); > event->sample.header.size = sizeof(struct perf_event_header); > > + if (!etm->timeless_decoding) > + sample.time = etm->latest_kernel_timestamp; > sample.ip = addr; > sample.pid = tidq->pid; > sample.tid = tidq->tid; > @@ -1248,6 +1251,8 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq, > event->sample.header.misc = cs_etm__cpu_mode(etmq, ip); > event->sample.header.size = sizeof(struct perf_event_header); > > + if (!etm->timeless_decoding) > + sample.time = etm->latest_kernel_timestamp; > sample.ip = ip; > sample.pid = tidq->pid; > sample.tid = tidq->tid; > @@ -2412,9 +2417,10 @@ static int cs_etm__process_event(struct perf_session *session, > else if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE) > return cs_etm__process_switch_cpu_wide(etm, event); > > - if (!etm->timeless_decoding && > - event->header.type == PERF_RECORD_AUX) Might want to add a comment here , /* * Record the latest kernel timestamp available in the header * for samples. */ > + if (!etm->timeless_decoding && event->header.type == PERF_RECORD_AUX) { > + etm->latest_kernel_timestamp = sample_kernel_timestamp; > return cs_etm__process_queues(etm); > + } > > return 0; > } > This is the best effort thing we could do to get things working. With the comments addressed: Acked-by: Suzuki K Poulos _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel