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, 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 812ABC433ED for ; Fri, 16 Apr 2021 10:18:41 +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 DDD556023E for ; Fri, 16 Apr 2021 10:18:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DDD556023E 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=HZmC/u0smN+hS57EyrH9n+wE3lIuN9/+oZ+OtWapGyk=; b=Xesdd4d4j9wZZXzCgjUwacqrD z8ZMO29SqoLDIM5o8zR/BuOF+VjiiURbXg+zw2ErnzxIvrdDYQo1O5nuTrD1iphNB5zoEHZ1RXLR0 AfH6Z2MVFVR4Z/esr6h5HJigu5wwSOh3yq/U/NdDuzybHch0AUpi23T8dNiQRDxzmoNJaR5/ZIaJe iUdbWmEmVmKUlkr4wzHJkDngxlAyBdOp0W3iBM6+yqvzV4SKUL6wCFKx6zYnzygfORfhcF6NzdJf3 Ii/xy9dJoWCNrw/qyHgwlKYVijy27+oNEew00IMc4aoyf6X+glxZa+5sl/MXY+MUPogGl/mt0bOEV QKBn0457A==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lXLWv-001i02-GS; Fri, 16 Apr 2021 10:16:53 +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 1lXLWp-001hyq-2W for linux-arm-kernel@desiato.infradead.org; Fri, 16 Apr 2021 10:16:50 +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=LTM+KdPJas7Ces/YmtM1zK7HvU9UPmaHRnBCUGKqB4E=; b=s0hfqQcHkc0Wrl77uS1p7X6W4I fmtyA0+ZU3aniEa9U71xio6QhhoudVs4z9pDEquAdc0OaHRt44u7U6P0EnPijE2MVRunj2KdXP9GA HzoA9UTrcwHpiLbAkVJ0LZjCkrxf8pwsfdiKsPtsIeaJTRlHYqyJzGpSXRLuPG8BQBIuqIsReiFVX V17yiKrgCUE4awafo2wkuQp6o5UYRpXg72o9lb3t5sd9018pgWjB3slw54pI/RNLL8nUY8NQdNeEN 56waXIAFPNw0I0T3xTtc7TBD0ILaTycs5p2OzJ/w6wxbpeFwgwy+iS9bz//HqPaIJUZf0qTZrHul5 4BPmgnZQ==; Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lXLWl-009GvZ-Tv for linux-arm-kernel@lists.infradead.org; Fri, 16 Apr 2021 10:16:45 +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 A3BEB106F; Fri, 16 Apr 2021 03:16:39 -0700 (PDT) Received: from [10.57.57.112] (unknown [10.57.57.112]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 74FB03FA35; Fri, 16 Apr 2021 03:16:37 -0700 (PDT) Subject: Re: [PATCH 2/2] perf cs-etm: Set time on synthesised samples to preserve ordering To: Mathieu Poirier Cc: coresight@lists.linaro.org, al.grant@arm.com, branislav.rankov@arm.com, denik@chromium.org, suzuki.poulose@arm.com, Mike Leach , Leo Yan , 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 References: <20210414143919.12605-1-james.clark@arm.com> <20210414143919.12605-2-james.clark@arm.com> <20210415195416.GB937505@xps15> From: James Clark Message-ID: <0ca8c095-9165-bcd1-26bc-3e5d2191affb@arm.com> Date: Fri, 16 Apr 2021 13:16:36 +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: <20210415195416.GB937505@xps15> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210416_031644_100967_44A7A309 X-CRM114-Status: GOOD ( 28.38 ) 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 15/04/2021 22:54, Mathieu Poirier wrote: > On Wed, Apr 14, 2021 at 05:39:19PM +0300, 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. >> > > I understand the problem. Once again an issue caused by CS and the kernel > having a different view of time. > >> 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(). >> >> Signed-off-by: James Clark >> Co-developed-by: Al Grant >> Signed-off-by: Al Grant > > Suzuki is correct, your name has to appear after Al's. > >> --- >> 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) >> + if (!etm->timeless_decoding && event->header.type == PERF_RECORD_AUX) { >> + etm->latest_kernel_timestamp = sample_kernel_timestamp; > > It will be fun to fix this when 8.4 comes out but for now it's the best we've > got. > Thanks for the reviews Leo, Mathieu and Suzuki. Yes I think for 8.4 we can also do something very similar to Leo's "perf arm-spe: Bail out if the trace is later than perf event" patch where decoding is paused until the other events with later timestamps have been received. At the moment the CS decoding happens all at once. I will submit a new set with the fixes and better variable name. James > Reviewed-by: Mathieu Poirier > >> return cs_etm__process_queues(etm); >> + } >> >> return 0; >> } >> -- >> 2.28.0 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel