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 1D46FC433F5 for ; Sun, 13 Mar 2022 12:46:02 +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=nBG5qn0mgeixHRCTFykSZzeIfiiY1oAK1VEhpqlxBTk=; b=mZWTkfJNVMuNLv EXAo/WEWLS2VE6No/ToVvzOiOwp948freii7srMaUiZ78Jl+2iiTavlPyWq8xYRIRiuKO3k77D4La UT+QleDhDjWfwaTwtBdsrlAVxiYypbRe3R3qmW0kJevhC4guUCyhPZifmn09jahz8SJhhJU+uzblp HaLvcXdzDP6ndjs3mQd4znC4TdAgQq85sIl26PAKbLlokMUW4cbb/WYt9Ek4MZN3Zf1xoVqcrgch4 kD2NCH3wN6ot0wkWZf3AOf/AVimtLLw6Am2bMYmprIrcAX6FAudWm+/TR1qH6gaF0WDnzIkDjuDG6 VqGl2OwHVpFLKGVHclKg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nTNaS-002lh6-J2; Sun, 13 Mar 2022 12:44:40 +0000 Received: from mail-ej1-x62a.google.com ([2a00:1450:4864:20::62a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nTNaO-002lgJ-Cg for linux-arm-kernel@lists.infradead.org; Sun, 13 Mar 2022 12:44:38 +0000 Received: by mail-ej1-x62a.google.com with SMTP id bi12so28521957ejb.3 for ; Sun, 13 Mar 2022 05:44:35 -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=EJ1tBx1dhbkFDPqhVx95CHpZt6g0rmeL6lGzXpgultw=; b=zFM9BbgWlCI8Z3Ni6/YkVN/Uuvp03ahg6Fs1xdaBLu4raOaXbF64NdAkjc7h0NZUVa QcNR2GSSEiZvjX0PvIwOeAv8dCfWRuTQVcCF9p4zFxfQ/tQGMtTVurdkW0hP4562Kodn NO+C6OaHvmQ70LIqfCjKlmVwpvIyy+bPY5CIzlw5vyRHNPlRrpXq3NtIaDQTfDNt4IqH Zm6fT/RHmi//0HlUI1PL3ZMB8acBKyADsdDCX+FcyQhqQayXN9LhifGsaIsNrDXETKEq 9acm9UbFGTnXgHaMhiZDkVfcmo4Nq3R9lNJFBSynDHF8w1CsG0rK/DegKCZqPYOEk2da EDHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=EJ1tBx1dhbkFDPqhVx95CHpZt6g0rmeL6lGzXpgultw=; b=2ZjflZ3Bty8ElGWPJOMMheJdftsOwXLof3wDW5fTp3TiMFI2Nhqp54VDz7spnxP6gS TgSdnfN06Ui2XcUebRvXjlgSKUaXEuygO01XNp9pmBo2vsiX8DMvFtXV3lTZNcvKhOLF B4Wl+n01u/VMmlWPSw46eOpj/tuyhKesrF6GPjwVI+I/kQE7kiE6kfuGeoJslBZ71Wop rFgOjgEaL21DRA+HmODMxP112h4VqML+Yfi0a8NfVsQxWISnsHUxWvJOR1trgUjOldEK DyoGS4OyFTP4Tt9Jasfd5TMF+C578h1MvslVuRN4FkBKTd8C58vL1DKhY0eRgQqKmjFC TgEQ== X-Gm-Message-State: AOAM532ZIL5p91FUQyFjeRj71oEyYPBs9N5KwWWOwwk7jEeDY+gPaCxe YO1u3/BFgwLGX6TnK0G9WsoblA== X-Google-Smtp-Source: ABdhPJwWwEQ/BJrrSXJsPZOH2KWN8TEOzijKIe5tSjyPVPvwusH61MygNa6ACyUHmatt6h24pnliaQ== X-Received: by 2002:a17:906:53c7:b0:6ce:6f32:ce53 with SMTP id p7-20020a17090653c700b006ce6f32ce53mr15441881ejo.352.1647175474370; Sun, 13 Mar 2022 05:44:34 -0700 (PDT) Received: from leoy-ThinkPad-X240s ([104.245.96.34]) by smtp.gmail.com with ESMTPSA id z3-20020a056402274300b004169771bd91sm6299081edd.39.2022.03.13.05.44.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 13 Mar 2022 05:44:33 -0700 (PDT) Date: Sun, 13 Mar 2022 20:44:27 +0800 From: Leo Yan To: German Gomez Cc: Ali Saidi , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, benh@kernel.crashing.org, Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , John Garry , Will Deacon , Mathieu Poirier , James Clark , Andrew Kilroy , Jin Yao , Kajol Jain , Li Huafei Subject: Re: [PATCH v2 2/2] perf mem: Support HITM for when mem_lvl_num is used Message-ID: <20220313124427.GB143848@leoy-ThinkPad-X240s> References: <20220221224807.18172-1-alisaidi@amazon.com> <20220221224807.18172-2-alisaidi@amazon.com> 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-20220313_054436_689456_38220C30 X-CRM114-Status: GOOD ( 36.54 ) 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, Mar 02, 2022 at 03:39:04PM +0000, German Gomez wrote: > > On 21/02/2022 22:48, Ali Saidi wrote: > > Current code only support HITM statistics for last level cache (LLC) > > when mem_lvl encodes the level. On existing Arm64 machines there are as > > many as four levels cache and this change supports decoding l1, l2, and > > llc hits from the mem_lvl_num data. Given that the mem_lvl namespace is > > being deprecated take this opportunity to encode the neoverse data into > > mem_lvl_num. > > Since Neoverse is mentioned in the commit message, I think there should be a comment somewhere in the code as well. > > > For loads that hit in a the LLC snoop filter and are fullfilled from a > > higher level cache, it's not usually clear what the true level of the > > cache the data came from (i.e. a transfer from a core could come from > > it's L1 or L2). Instead of making an assumption of where the line came > > from, add support for incrementing HITM if the source is CACHE_ANY. > > > > Since other architectures don't seem to populate the mem_lvl_num field > > here there shouldn't be a change in functionality. > > > > Signed-off-by: Ali Saidi > > --- > > tools/perf/util/mem-events.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c > > index ed0ab838bcc5..6c3fd4aac7ae 100644 > > --- a/tools/perf/util/mem-events.c > > +++ b/tools/perf/util/mem-events.c > > @@ -485,6 +485,7 @@ int c2c_decode_stats(struct c2c_stats *stats, struct mem_info *mi) > > u64 daddr = mi->daddr.addr; > > u64 op = data_src->mem_op; > > u64 lvl = data_src->mem_lvl; > > + u64 lnum = data_src->mem_lvl_num; > > u64 snoop = data_src->mem_snoop; > > u64 lock = data_src->mem_lock; > > u64 blk = data_src->mem_blk; > > @@ -527,16 +528,18 @@ do { \ > > if (lvl & P(LVL, UNC)) stats->ld_uncache++; > > if (lvl & P(LVL, IO)) stats->ld_io++; > > if (lvl & P(LVL, LFB)) stats->ld_fbhit++; > > - if (lvl & P(LVL, L1 )) stats->ld_l1hit++; > > - if (lvl & P(LVL, L2 )) stats->ld_l2hit++; > > - if (lvl & P(LVL, L3 )) { > > + if (lvl & P(LVL, L1) || lnum == P(LVLNUM, L1)) > > + stats->ld_l1hit++; > > + if (lvl & P(LVL, L2) || lnum == P(LVLNUM, L2)) > > + stats->ld_l2hit++; It's good to split into two patches: one patch is to add statistics for field 'mem_lvl_num', the second patch is to handle HITM tags. > > + if (lvl & P(LVL, L3) || lnum == P(LVLNUM, L4)) { It's a bit weird that we take either PERF_MEM_LVL_L3 or PERF_MEM_LVLNUM_L4 as the last level local cache in the same condition checking. > According to a comment in the previous patch, using L4 is specific to Neoverse, right? > > Maybe we need to distinguish the Neoverse case from the generic one here as well > > if (is_neoverse) > // treat L4 as llc > else > // treat L3 as llc I personally think it's not good idea to distinguish platforms in the decoding code. To make more more clear statistics, we can firstly increment hit values for every level cache respectively; so we can consider to adde two extra statistics items 'stats->ld_l3hit' and 'stats->ld_l4hit'. if (lvl & P(LVL, L3) || lnum == P(LVLNUM, L3)) stats->ld_l3hit++; if (lnum == P(LVLNUM, L4)) stats->ld_l4hit++; > > if (snoop & P(SNOOP, HITM)) > > HITM_INC(lcl_hitm); > > else > > stats->ld_llchit++; For the statistics of 'ld_llchit' and 'lcl_hitm', please see below comment. > > } > > > > - if (lvl & P(LVL, LOC_RAM)) { > > + if (lvl & P(LVL, LOC_RAM) || lnum == P(LVLNUM, RAM)) { > > stats->lcl_dram++; > > if (snoop & P(SNOOP, HIT)) > > stats->ld_shared++; > > @@ -564,6 +567,9 @@ do { \ > > HITM_INC(rmt_hitm); > > } > > > > + if (lnum == P(LVLNUM, ANY_CACHE) && snoop & P(SNOOP, HITM)) > > + HITM_INC(lcl_hitm); > > + The condition checking of "lnum == P(LVLNUM, ANY_CACHE)" is a bit suspecious and it might be fragile for support multiple archs. So I am just wandering if it's possible that we add a new field 'llc_level' in the structure 'mem_info', we can initialize this field based on different memory hardware events (e.g. Intel mem event, Arm SPE, etc). During the decoding phase, the local last level cache is dynamically set to 'mem_info:: llc_level', we can base on it to increment 'ld_llchit' and 'lcl_hitm', the code is like below: if ((lvl & P(LVL, REM_CCE1)) || (lvl & P(LVL, REM_CCE2)) || mrem) { if (snoop & P(SNOOP, HIT)) stats->rmt_hit++; else if (snoop & P(SNOOP, HITM)) HITM_INC(rmt_hitm); + } else { + if ((snoop & P(SNOOP, HIT)) && (lnum == mi->llc_level)) + stats->ld_llchit++; + else if (snoop & P(SNOOP, HITM)) + HITM_INC(lcl_hitm); } Thanks, Leo > > if ((lvl & P(LVL, MISS))) > > stats->ld_miss++; > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel