From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6E9D138AC68; Fri, 25 Sep 2026 15:46:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790351169; cv=none; b=D8V2W5rNZt5dMoYlM+H63/NksWzbjkFeUEBwLVZMACwdb6jG2IXq9G7ipXnyZ6aosjkgUM8uYF0LnoohBAhiknPs66cBymVSSKJkZieSmS7yimj7Nw7DNWs4fbqKXbmfTrK/pvhJpQi3aoas0Xwl0tszGb4w1aVHvqqe2THFrUo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790351169; c=relaxed/simple; bh=yYhF0T0r2ejlFAcIDQOqW/jCyY0rZUE+7Keg1M1WgAQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sJ63YnGEPJOUc6SxtuwcaChhsUAABl2PRMWJkQG+uYlp7l3NvMNkWDsqkBPSksta5VnoLuIXJIUnyZn9lCeO0fTKlXGEAHKOsXmZZgvy59X0y6uYXketg9fRzh6Lv5Y7uXyJbK9f7a7P8FEvCL2tF9VDDWVwxbmWjNyFI5A+8BQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=PwMaMO+Z; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="PwMaMO+Z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 83DF11F000FF; Fri, 25 Sep 2026 15:46:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790351167; bh=f2EOEeTaYY/03hTFnzqTJySuidDAmR79fMzTSBUU+GA=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=PwMaMO+Zw2A33zQHJyJASlVaM+/HexwofPzX0Dah3j2jLoSISP03rj10KjCP4LQ/D Q2D81EQnr12zAmIPWWtSr8VRgnJGWqeNr+FTg1HZVYcRuHzfD49DdQX3ivVs7htncN lLFWQKU6yAbx0eoq3DBc44Ywjn7PU12/fz8nw+jL6v5eUVMd51OjtgrISRPdEMVQyd wCfAplFxiOCDtsLFONDSmnGe/i8zaAOVDV2kjVL7Nkc7bmCf0dpPQiWo2NecTRLkQX qjjoOpoPnNAusyB+NeNAYTP2+2bZHWMuGyhqIEnxEXfbwodZqBAt2cjP+qVFunPFaD Rf+Ut5Dyrk6gg== Date: Fri, 25 Sep 2026 08:46:05 -0700 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Thomas Gleixner , James Clark , Jiri Olsa , Ian Rogers , Adrian Hunter , Clark Williams , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Masami Hiramatsu Subject: Re: [PATCH v5 0/6] perf annotate-data: Fix hangs on broken debug info, AMD mem record Message-ID: References: <20260925150657.1826942-1-acme@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260925150657.1826942-1-acme@kernel.org> On Fri, Sep 25, 2026 at 05:06:51PM +0200, Arnaldo Carvalho de Melo wrote: > Hi, > > This originated in another patch series, 'perf tools: Annotate fixes, > stdio progress indication, debuginfo-client in more places', and is > being split off so that these fixes can be reviewed right away; the > debuginfod download feature work from it will come later, separately, > based on this series. > > - 'perf report -s type' spins forever on the dwz compressed debug info of > zlib-ng (libz.so.1): die_collect_vars() saves a type DIE offset that is > relative to the file the DIE lives in, the dwz alt file for types shared > by several CUs, and resolving it in the main file parses whatever is at > that offset, here a typedef whose DW_AT_type refers to itself, making the > typedef/qualifier chase spin (patch 3), with the chases bounded so that > other kinds of broken debug info don't hang perf either (patches 1, 2 > and 4); > > - 'perf mem record' requests PERF_SAMPLE_CPU (patch 5) and uses the IBS > swfilt filter when the kernel exposes it (patch 6), with the tables > carrying the term kept in the arch/x86 code, where the knowledge that > IBS needs it stays, as Ravi Bangoria suggested reviewing Namhyung > Kim's v6 review remark on this patch > (<4349c387-5b8a-4e8d-932a-5175a7598e1e@amd.com>, replying to > ). > > About PATCH 5, answering Namhyung Kim's v6 review question > () about the --sample-cpu default: the > data source field is about the memory hierarchy level of the access, > it has no record of which CPU issued it, and while the TID is in > every sample and in the CTF stream, a thread time-sliced on one CPU > or moved between SMT siblings is not told apart by it from cross-core > contention, so it is the CPU id that keys it, and it is what the > false-sharing detector in pahole needs. The TID is recorded as well. > > Requires elfutils 0.160 for dwarf_cu_getdwarf(), so the libdw feature test > probes for it and Makefile.config says 0.160: older versions now disable > dwarf support with that message instead of failing to link. I have a nitpick on the patch 4, but otherwise looks good to me. Reviewed-by: Namhyung Kim Thanks, Namhyung > > Best regards, > > - Arnaldo > > What changed from v4: > > - Added Ravi Bangoria's Reviewed-by (<76ef085b-e8e3-41a5-b029-2cd0489aa430@amd.com>), > given together with his rationale for keeping the IBS swfilt selection > explicit: 'perf record' fails rather than transparently retrying with > /swfilt=1/, so that the user is aware software filtering is being used > and that it is not overhead free. > - Trimmed the comment on the AMD mem event tables in > tools/perf/arch/x86/util/mem-events.c per that review, dropping its > last sentence about how 'perf record' handles exclude bits on open > failure; the code is unchanged. > - Patch 4: reworded the comment on the truncated flag; the JSON exporter > that reads it is added in a later series. > - Patch 6: dropped a leftover no-op hunk from tools/perf/util/mem-events.c; > the swfilt selection is all in the arch/x86 code. > Command to see this delta: git diff eb568956a7129c91..HEAD > > What changed from v3: > > - Dropped patch 5/7, "perf annotate-data: Show the sample count in the > data-type browser": it is already in perf-tools-next as commit > 9db4e9d6cc7c, so the series is now 6 patches and the subject no longer > mentions it. > - Rebased onto current perf-tools-next (base-commit below; v3 was based on > 29f320d221c1), patch 4 adapted to the upstream __add_member_cb() cleanup > that dropped the member_type local, no behavior change. > > What changed from v2: > > Only aggregate (struct/union) members are marked as truncated when the > MAX_MEMBER_DEPTH limit is reached in the member nesting recursion (patch > 4), addressing a sashiko [Medium] review finding > (<20260921165704.38DBD1F000FF@smtp.kernel.org>): the v2 check was made > before looking at the member's type, so a primitive field that merely > landed on the limit, e.g. an int in a struct nested 31 deep, was marked > as truncated and logged even though it has no children to expand. The > limit still bounds the recursion, as only aggregates are expanded. > > What changed from v1: > > Bump MAX_MEMBER_DEPTH from 8 to 32, addressing a review comment from Namhyung. > > tools/build/feature/test-libdw.c | 10 ++- > tools/perf/Documentation/perf-mem.txt | 4 + > tools/perf/Makefile.config | 2 +- > tools/perf/arch/x86/util/mem-events.c | 17 ++++ > tools/perf/arch/x86/util/mem-events.h | 2 + > tools/perf/arch/x86/util/pmu.c | 10 ++- > tools/perf/builtin-mem.c | 7 +- > tools/perf/tests/shell/test_data_symbol.sh | 6 +- > tools/perf/util/annotate-data.c | 41 +++++++-- > tools/perf/util/annotate-data.h | 3 + > tools/perf/util/dwarf-aux.c | 140 ++++++++++++++++++++++++----- > tools/perf/util/dwarf-aux.h | 13 +++ > 12 files changed, 217 insertions(+), 38 deletions(-) > > v4-head: eb568956a7129c91a8c0ccebb5979440a046c7ef > v3-head: e2504280f5c4086e9851a758ed1bae8df7ef269e > base-commit: edd8a9fe2eca009599e013a29c421c7a6b5ad1b9 > --