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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 4B6C0C77B7E for ; Sat, 27 May 2023 09:07:17 +0000 (UTC) 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: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=Fg6eTGKrHdueP/KhBZlphgusB9WdxC2lrmAeTFetaC0=; b=sTIMQk6WCu2Fe/ jmCMP7wbhU7Hx8IxAqu8rXMGG0G0vgovgN0p2zmnx8BJgSzRS7ZJ9PcEAwEQaWos+cBQo4YbBoHLB /GeDFXPAi7z7YA1+tE+d/Rd8f9tZFgrf1UKlv3jabZTlfapHfYNh3XRKfIbUneifGosDFUk6NxH5a EQnDDWc8zvlmOYMluLlRlTrTRUfw04dFohWudA8fnUCmW9Rc7UJFKAqNLfni7r6NeWc46jAJ597JJ 2bFoBRQdA+/W3a9EwPv0X/Rzsths1gzJEJpk40DiCS2f7Ub17KVjVhPsociLhTx3PemjdRdRt/ob8 1RuJTHRooym/OYoQpiYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q2psw-005JIE-2q; Sat, 27 May 2023 09:06:50 +0000 Received: from mail-pf1-x42c.google.com ([2607:f8b0:4864:20::42c]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q2pst-005JGL-2k for linux-arm-kernel@lists.infradead.org; Sat, 27 May 2023 09:06:49 +0000 Received: by mail-pf1-x42c.google.com with SMTP id d2e1a72fcca58-64d3e5e5980so2106243b3a.2 for ; Sat, 27 May 2023 02:06:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1685178405; x=1687770405; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Th/2CdkYSffxqn+a5kfATTdZzNBhiqjv8/1oEFRsaQ8=; b=swpKci/XFYc82knSogXS3iRF6oaRbVxcKOC3km53iIFTc4IUoWmJLA5Ym/gTJVjirQ IDmaUDmh3mK9Wgg4abfbgEMHihj1W3Kpb+XX16Supshd9Y1UFFRZapWsGbJ4eWUIKNnP vXKFUU5W+Tl01eJeicZII5BFwIAhq2c5wgctdvDywSvxO519UV5GeX/wcfKFeD/ylHTp 97iv6To8vC0uc9iuhm1vXbUh3GrikszWBxu7CfJ3fi/ON5ktra8+b0GgLomyoGJPL/rY HKICfPdWZxQj6nKtsquTYDZLzpFPo5XqhtKGwq+5VR944qkR2hI6ygkiygv2icjPnq6P EULQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685178405; x=1687770405; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Th/2CdkYSffxqn+a5kfATTdZzNBhiqjv8/1oEFRsaQ8=; b=KHY0zxB6vf9RL//uDl9PGZeHCmWYOq3mOoP7cPLBMvc/btHrQg3tUmxL3cOWJdfOEh 582xGDJPK22MPMQfXGvFewBdGWGCIfaLNj5Vsn78LI7alVBpnFjdf0AnzFC1kr2lvept SmvyGNsOUzptEE6GDcYjSCN9cQ0xxKCsIr37KAqCG8jgNVNB8Uj0D0J/Bo320lVYC2wZ GlQk98qBngsZcBNr9cU4PJgidAbEzrGjlXkoLYsO0CRO9SQOH+odKUi4VB562YMTpnpo /oe8mg23R3mJnhYFSAEZrwiX2LKOQcsw/acwQvR6ww0+xsWG+J00E64ePFGYENXdAXnf hvQQ== X-Gm-Message-State: AC+VfDyD06+6JTljNeu9RCZfifqyk717w0hff2n7cKv7fUCDEl91ZXcs W0XnRrZNxdwOBufL4+nJOYBJ0g== X-Google-Smtp-Source: ACHHUZ63xJO8hiOlOALtVX+qtnyTR/xYlZeXvYN1YQQY4IJsTzbGd/KqW7Hj7EUIzCZiXPdl/kM2Jg== X-Received: by 2002:a05:6a00:891:b0:643:d40c:7db1 with SMTP id q17-20020a056a00089100b00643d40c7db1mr6866644pfj.3.1685178404926; Sat, 27 May 2023 02:06:44 -0700 (PDT) Received: from leoy-yangtze.lan ([107.151.177.126]) by smtp.gmail.com with ESMTPSA id j12-20020aa78dcc000000b0064928cb5f03sm3797231pfr.69.2023.05.27.02.06.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 27 May 2023 02:06:44 -0700 (PDT) Date: Sat, 27 May 2023 17:06:35 +0800 From: Leo Yan To: James Clark Cc: coresight@lists.linaro.org, denik@chromium.org, Suzuki K Poulose , Mike Leach , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , John Garry , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/4] perf cs-etm: Use previous thread for branch sample source IP Message-ID: <20230527090635.GB886420@leoy-yangtze.lan> References: <20230524131958.2139331-1-james.clark@arm.com> <20230524131958.2139331-3-james.clark@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230524131958.2139331-3-james.clark@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230527_020647_982642_973CC3EA X-CRM114-Status: GOOD ( 24.01 ) 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 Wed, May 24, 2023 at 02:19:56PM +0100, James Clark wrote: > Branch samples currently use the IP of the previous packet as the from > IP, and the IP of the current packet as the to IP. But it incorrectly > uses the current thread. In some cases like a jump into a different > exception level this will attribute to the incorrect process. It's about the timing that branch has taken or not taken :) If we think the branch sample as 'branch has taken', then current code is doning right thing, otherwise, we need this fix. > Fix it by tracking the previous thread in the same way the previous > packet is tracked. > > Signed-off-by: James Clark > --- > tools/perf/util/cs-etm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index ebffc9052561..a997fe79d458 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -86,6 +86,7 @@ struct cs_etm_traceid_queue { > size_t last_branch_pos; > union perf_event *event_buf; > struct thread *thread; > + struct thread *prev_thread; > struct branch_stack *last_branch; > struct branch_stack *last_branch_rb; > struct cs_etm_packet *prev_packet; > @@ -480,6 +481,7 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq, > tidq->trace_chan_id = trace_chan_id; > tidq->thread = machine__findnew_thread(&etm->session->machines.host, -1, > queue->tid); > + tidq->prev_thread = machine__idle_thread(&etm->session->machines.host); > > tidq->packet = zalloc(sizeof(struct cs_etm_packet)); > if (!tidq->packet) > @@ -616,6 +618,8 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm, > tmp = tidq->packet; > tidq->packet = tidq->prev_packet; > tidq->prev_packet = tmp; > + thread__put(tidq->prev_thread); > + tidq->prev_thread = thread__get(tidq->thread); Maybe cs_etm__packet_swap() is not the best place to update "tidq->prev_thread", since swapping packet doesn't mean it's necessarily thread switching; can we move this change into the cs_etm__set_thread()? Thanks, Leo > } > } > > @@ -791,6 +795,7 @@ static void cs_etm__free_traceid_queues(struct cs_etm_queue *etmq) > /* Free this traceid_queue from the array */ > tidq = etmq->traceid_queues[idx]; > thread__zput(tidq->thread); > + thread__zput(tidq->prev_thread); > zfree(&tidq->event_buf); > zfree(&tidq->last_branch); > zfree(&tidq->last_branch_rb); > @@ -1450,8 +1455,8 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq, > sample.time = cs_etm__resolve_sample_time(etmq, tidq); > > sample.ip = ip; > - sample.pid = tidq->thread->pid_; > - sample.tid = tidq->thread->tid; > + sample.pid = tidq->prev_thread->pid_; > + sample.tid = tidq->prev_thread->tid; > sample.addr = cs_etm__first_executed_instr(tidq->packet); > sample.id = etmq->etm->branches_id; > sample.stream_id = etmq->etm->branches_id; > -- > 2.34.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel