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 DE0A1C7EE25 for ; Thu, 8 Jun 2023 10:26:29 +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=ythdzYpEy4OpXckfBubNvryvh2CiuZf8S52MMtktcaY=; b=nNOkpjUL9k+CDK wiELTXtHRPyC0wI6pgafVBiXoI4j+ZTduImrNFINUx+Te5Bh0/dDZuXTjMOUMHYEnvGm6Ft1uvMqH KVhQpU26ueU/Awihk84nura317Td/sc9NtFsvauECrsPf8U2GOta+yAuV2iarQYxHb2SNO+pMOjTK PPA+BYbsmlS82LxMXO8fvtP/euRiSDnjQVq9Fnqqa5rRLVXUWTFg+nj10mQzicPpoJzDjNORNvgYJ 9qcdSS3gs16xRgUsQLIpVpLqQG3HUNtZgkLal11qIOT/ZHOxvKnUTwUtmpBgxchPN/WFe1N2xjonI Z8BachBvwWfx4syaEczQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q7CqF-008pr7-20; Thu, 08 Jun 2023 10:26:07 +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 1q7CqD-008pqA-1J for linux-arm-kernel@lists.infradead.org; Thu, 08 Jun 2023 10:26:06 +0000 Received: by mail-pf1-x42c.google.com with SMTP id d2e1a72fcca58-652d76be8c2so367124b3a.3 for ; Thu, 08 Jun 2023 03:26:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1686219962; x=1688811962; 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=3xkH3r1pvwgUrEo7Gtm0dIWmSa6nSh/lG9Vqe1Of73g=; b=nCZY15shRf0+YCOaFOuY8X6w310iPAZUYi5ASkQz/GUc+4dSc2cReU0+52jpmuft1b zgRskJqRB9fkL8XXnZXtSGXpDEiQT3BsO1De6EkspTxOfkC7L1/My9JygCq3fEI5DVi3 gIZ8n1z9SzT8oR85zl9AJLdkOhhKpzMKA38CfDUFMNh3BPtoHjzUjRbbrn29AMGqT0F/ zu2wsnbm1hwaE4pjxWEmVcbQkkPkVGvF5UzRSs4IG0HYp0380HCk0LZjOxYvbLkanv/1 2eahg6XTZb7T4cCEZtQIc/e0qfI3USBtTHh1PlSGPKru2u9a+yQrVvaFuDoJ9Ic/yvd4 s/zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686219962; x=1688811962; 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=3xkH3r1pvwgUrEo7Gtm0dIWmSa6nSh/lG9Vqe1Of73g=; b=EWuEDRTgZsMbSm3B1PUrmLYm3oGUnvkMyzdZZi2dNlqgQBiOyatOz6kRrFc6YsRrR+ NBodUKwGdap/x83D/b9GtE84/miMM6kUnFVAaCcBQquW7V7OIMLQIxpOMD6GysFQmoUU XSGYNU8/Mk0IwW2HeIQqvchHyUs2Ygw2ztyO3qNhVkqsZfKrzyOsXlRj+uMAxk+Vv6Ow klQw5iNYQjapefUkgiLyOTke4RgTdFUS7TZ9G7N6+ExbLgGcERl7skyLXlY06z2PNEwX YXo3CSnYRuKJ43ao3dePwPwQfJxxJb62ldKRUyajNDXZb6/b+JPU0couTUjEKuiNDQDF OUFw== X-Gm-Message-State: AC+VfDxDg32MLpuP+EuOmmo9Ikbxqrw0ycUC501cg802/lSH9ktGSdtq sW18mTGatXEzkmjuenBbkTkyNQ== X-Google-Smtp-Source: ACHHUZ5JTCRE7fcyNWts+H2vn0LEXWHBO6ClckfueGh8B+IzoyS9r/drZKEBPVRJZZdzupk6aKns1w== X-Received: by 2002:a05:6a00:3a26:b0:659:3031:5037 with SMTP id fj38-20020a056a003a2600b0065930315037mr9799968pfb.3.1686219962556; Thu, 08 Jun 2023 03:26:02 -0700 (PDT) Received: from leoy-huanghe ([156.59.39.101]) by smtp.gmail.com with ESMTPSA id m23-20020aa79017000000b006410f4f3ecdsm822251pfo.83.2023.06.08.03.25.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 03:26:02 -0700 (PDT) Date: Thu, 8 Jun 2023 18:25:55 +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: <20230608102555.GB123723@leoy-huanghe> References: <20230524131958.2139331-1-james.clark@arm.com> <20230524131958.2139331-3-james.clark@arm.com> <20230527090635.GB886420@leoy-yangtze.lan> <630ab636-107d-4b12-5454-2ee91ad43543@arm.com> <1e7aa657-6d1e-9e7c-95cb-b32d307abe93@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1e7aa657-6d1e-9e7c-95cb-b32d307abe93@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230608_032605_514564_7A2D6692 X-CRM114-Status: GOOD ( 25.04 ) 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 Thu, Jun 08, 2023 at 10:34:42AM +0100, James Clark wrote: [...] > >>> @@ -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()? > >> > > > > Yeah that might make more sense. I can move it there if we decide to > > keep this change. > > > > Unfortunately I don't think I can make this change. It seems like > putting the previous thread swap in cs_etm__set_thread() has different > semantics to keeping all the swaps together in cs_etm__packet_swap(). Thanks for trying this. > This is because if you swap the thread in cs_etm__packet_swap() the > previous packet and next packet can have the _same_ thread if there > happened to be no change. However if you only swap previous thread in > cs_etm__set_thread(), that means that the previous thread is always > different to the next one. This has a huge difference on the decoding > because two adjacent packets on the same thread will say they branched > from the previous thread that ran, not the previous thread on the > previous packet. Seems to me, this is a synchronization issue between the field 'tidq->prev_thread' and 'tidq->prev_packet'. It's still hard for me to understand "two adjacent packets on the same thread will say they branched from the previous thread that ran", IIUC, even we move thread swapping into cs_etm__set_thread(), if the two adjacent packets are in the same thread context, we can skip to update fields 'tidq->prev_thread' and 'tidq->prev_packet'. So I am curious if below cs_etm__set_thread() works or not? static void cs_etm__set_thread(struct cs_etm_auxtrace *etm, struct cs_etm_traceid_queue *tidq, pid_t tid) { struct machine *machine = &etm->session->machines.host; /* No context switching, bail out */ if ((tidq->thread->tid != tid) return; /* If tid is -1, we simply use idle thread context */ if (tid == -1) goto find_idle_thread; /* * The new incoming tid is different from current thread, * so it's to switch to the next thread context. */ /* Swap thread contexts */ thread__put(tidq->prev_thread); tidq->prev_thread = thread__get(tidq->thread); /* Find thread context for new tid */ thread__zput(tidq->thread); tidq->thread = machine__find_thread(machine, -1, tid); find_idle_thread: /* Couldn't find a known thread */ if (!tidq->thread) tidq->thread = machine__idle_thread(machine); } Thanks, Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel