Message ID | 20231129060211.1890454-1-irogers@google.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a5a7:0:b0:403:3b70:6f57 with SMTP id d7csp143753vqn; Tue, 28 Nov 2023 22:02:35 -0800 (PST) X-Google-Smtp-Source: AGHT+IHzk058/xEl8vIkFbFxR6RsQx9FhQIdxhEwgoCYKlq1R00xBX5LkWFm011rIAF6Jut78lXu X-Received: by 2002:a17:902:9345:b0:1cf:b597:584 with SMTP id g5-20020a170902934500b001cfb5970584mr12661346plp.41.1701237755356; Tue, 28 Nov 2023 22:02:35 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701237755; cv=none; d=google.com; s=arc-20160816; b=O1eAE0VicsjFZkcJMf0Q087Wfco7Z6LaA5Zme365czyapcyoPcfxc6l/ukXzn2D1+P 2APAeADRHqPfX1CnNDi9/Or0kP34YDlbkOPk90wvPpqWQG+LnbCDujj9ykpUtHnq2Nuu R3RtnQ2y+ukanwyujuTTSY2UDvQ9au/CswXqX5BWzuL5V4jNGc4okbe/hSt4RcRFPLTT XqBg8Q7EG5GNp5I6laHBFdLUac6O3wuD9/iZSgSSvag2vGKFuTI7N8b0OudV5genC82L Cdt+IMe7PE+uxLkK7yNZx4qkjkz1cfvtLQ47eFJoMw3v5rLcd74p2tMEOh+GLvuJw763 QiSQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:to:from:subject:mime-version:message-id:date :dkim-signature; bh=M0G+iPFdqPEFrwKvYIt+aM1SI2CrgzPYcwp9sXOSF54=; fh=uYKNI4aCUAMYZ/8oQVBIse94GgoxWIIZr1zaTbrWG7c=; b=mbRSynYRrS86x6YhISkAqwxjfj2UjrWay8Ic8bL5X7QadyI/09JpA2yAjvynsxv9Z5 6bWtkY8BTHlTWevBmFaXfB/j4CNViEbyHI55tpqN1wvPIBsERLtSUZyH4JbGoKn47gBt Hfb0xWcvg0SyVEySjuHrsY87W5JE6d5I3bb68bjdX/5/RrQtmn6Qb3NcJDpFTI6nvQuO WaAPtdIWOaHXOVS9xwW545hv8nebY4gj8LqHAJ7SabEcKwtCbD2tyXyeiW6I4o1PIG4h 0Qt6OYKfHs2dpSLl4bD0TPtKY/jIjBlWF4zl4An92klUERWD2Tv6E1zAbLO0nXVed5HF /9Fg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=F43Cdyei; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id p2-20020a170902e74200b001cfd52a2266si5372112plf.403.2023.11.28.22.02.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Nov 2023 22:02:35 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=F43Cdyei; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id E4DDD804EE70; Tue, 28 Nov 2023 22:02:31 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1376992AbjK2GCL (ORCPT <rfc822;kernel.ruili@gmail.com> + 99 others); Wed, 29 Nov 2023 01:02:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45802 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234848AbjK2GCJ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 29 Nov 2023 01:02:09 -0500 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 917CD19AE for <linux-kernel@vger.kernel.org>; Tue, 28 Nov 2023 22:02:15 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id 3f1490d57ef6-db3fc4a1254so7689609276.3 for <linux-kernel@vger.kernel.org>; Tue, 28 Nov 2023 22:02:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701237735; x=1701842535; darn=vger.kernel.org; h=to:from:subject:mime-version:message-id:date:from:to:cc:subject :date:message-id:reply-to; bh=M0G+iPFdqPEFrwKvYIt+aM1SI2CrgzPYcwp9sXOSF54=; b=F43Cdyei04AdOpfSQd3OpAJLSm9xhRahe1O9QqkF0mPs+iLhQVMaGKePv7pinYZ8Tt 0wvo1siGlfX7V9OTI2E1LDcrKbupla3Oq1lkJ9v3JRvjB/+Z5gDI6OAfI7dgs+FN5cE6 Hjx4ZL1f7OMt0sQXCWHzs+0jBc4NqNejl8NeWqOYRUlrJOu71IZ0zvvaojVqYF96RQUV fBz+l7xylyjOOwEaNhN8igDLxh1OHIortZ8drWSn4OsasC7ouRfbL1ihY0d2nc0kbL9J 7Qb42/47N4NEToqDAXQRLXwX/AT3ivOJJ6+RvfdawBEMIyP2+qMNC69Fw5BO9I5kQPL9 yahw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701237735; x=1701842535; h=to:from:subject:mime-version:message-id:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=M0G+iPFdqPEFrwKvYIt+aM1SI2CrgzPYcwp9sXOSF54=; b=s+pnFyxd6HHRE4dloOeixSf8XI1G4wHHnx16cjTjNJlGON4K4h0N5WECtITZTMeMcu gTycbKvaKO3+XvT4rUS5d73qZd5wZjJ8DyEZ2frUaoyJiuL4/zRWedkD+sBwLtIpPgkp 7pH8jmW7j7mMoJa6y+tk7ftCif/i2v/X2zAV9Dr7VkfDCfvBLvztLtc409RCy8w2XmNV 2YqJ+oyVuZd3htZqR9TRmU01wRuK0WgG0GRIs8BuX08FQHtJ20SeItrnV8wKzwVosSDA fM7S9ncDeH/vMYhWxmWGLl6nAt+KZi9l/f16P5TU/s1pcUFaIuuutb4mA/OetWHRr5cj Z0Yg== X-Gm-Message-State: AOJu0Yw9X6alczuQ36SIbGwsMhA92JYRGP0fKcYxCFn/0bUB7adJ19Yf BPlvdWbV6jv+IJoCUS3BmLjl3ak6JIx/ X-Received: from irogers.svl.corp.google.com ([2620:15c:2a3:200:763b:80fa:23ca:96f8]) (user=irogers job=sendgmr) by 2002:a25:d114:0:b0:da3:ab41:304a with SMTP id i20-20020a25d114000000b00da3ab41304amr505879ybg.4.1701237734702; Tue, 28 Nov 2023 22:02:14 -0800 (PST) Date: Tue, 28 Nov 2023 22:01:57 -0800 Message-Id: <20231129060211.1890454-1-irogers@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.43.0.rc1.413.gea7ed67945-goog Subject: [PATCH v1 00/14] Clean up libperf cpumap's empty function From: Ian Rogers <irogers@google.com> To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Mike Leach <mike.leach@linaro.org>, James Clark <james.clark@arm.com>, Leo Yan <leo.yan@linaro.org>, John Garry <john.g.garry@oracle.com>, Will Deacon <will@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Darren Hart <dvhart@infradead.org>, Davidlohr Bueso <dave@stgolabs.net>, " =?utf-8?q?Andr=C3=A9_Almeida?= " <andrealmeid@igalia.com>, Kan Liang <kan.liang@linux.intel.com>, K Prateek Nayak <kprateek.nayak@amd.com>, Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com>, Kajol Jain <kjain@linux.ibm.com>, Athira Rajeev <atrajeev@linux.vnet.ibm.com>, Andrew Jones <ajones@ventanamicro.com>, Alexandre Ghiti <alexghiti@rivosinc.com>, Atish Patra <atishp@rivosinc.com>, "Steinar H. Gunderson" <sesse@google.com>, Yang Jihong <yangjihong1@huawei.com>, Yang Li <yang.lee@linux.alibaba.com>, Changbin Du <changbin.du@huawei.com>, Sandipan Das <sandipan.das@amd.com>, Ravi Bangoria <ravi.bangoria@amd.com>, Paran Lee <p4ranlee@gmail.com>, Nick Desaulniers <ndesaulniers@google.com>, Huacai Chen <chenhuacai@kernel.org>, Yanteng Si <siyanteng@loongson.cn>, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, bpf@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Tue, 28 Nov 2023 22:02:32 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783877080455960777 X-GMAIL-MSGID: 1783877080455960777 |
Series |
Clean up libperf cpumap's empty function
|
|
Message
Ian Rogers
Nov. 29, 2023, 6:01 a.m. UTC
Rename and clean up the use of libperf CPU map functions particularly focussing on perf_cpu_map__empty that may return true for maps containing CPUs but also with an "any CPU"/dummy value. perf_cpu_map__nr is also troubling in that iterating an empty CPU map will yield the "any CPU"/dummy value. Reduce the appearance of some calls to this by using the perf_cpu_map__for_each_cpu macro. Ian Rogers (14): libperf cpumap: Rename perf_cpu_map__dummy_new libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new libperf cpumap: Rename perf_cpu_map__empty libperf cpumap: Replace usage of perf_cpu_map__new(NULL) libperf cpumap: Add for_each_cpu that skips the "any CPU" case libperf cpumap: Add any, empty and min helpers perf arm-spe/cs-etm: Directly iterate CPU maps perf intel-pt/intel-bts: Switch perf_cpu_map__has_any_cpu_or_is_empty use perf cpumap: Clean up use of perf_cpu_map__has_any_cpu_or_is_empty perf top: Avoid repeated function calls perf arm64 header: Remove unnecessary CPU map get and put perf stat: Remove duplicate cpus_map_matched function perf cpumap: Use perf_cpu_map__for_each_cpu when possible libperf cpumap: Document perf_cpu_map__nr's behavior .../perf/Documentation/examples/sampling.c | 2 +- .../perf/Documentation/libperf-sampling.txt | 2 +- tools/lib/perf/Documentation/libperf.txt | 4 +- tools/lib/perf/cpumap.c | 92 +++++++++++++------ tools/lib/perf/evlist.c | 6 +- tools/lib/perf/evsel.c | 2 +- tools/lib/perf/include/perf/cpumap.h | 56 ++++++++++- tools/lib/perf/libperf.map | 10 +- tools/lib/perf/tests/test-cpumap.c | 4 +- tools/lib/perf/tests/test-evlist.c | 6 +- tools/lib/perf/tests/test-evsel.c | 2 +- tools/perf/arch/arm/util/cs-etm.c | 83 +++++++---------- tools/perf/arch/arm64/util/arm-spe.c | 4 +- tools/perf/arch/arm64/util/header.c | 15 +-- tools/perf/arch/x86/util/intel-bts.c | 4 +- tools/perf/arch/x86/util/intel-pt.c | 10 +- tools/perf/bench/epoll-ctl.c | 2 +- tools/perf/bench/epoll-wait.c | 2 +- tools/perf/bench/futex-hash.c | 2 +- tools/perf/bench/futex-lock-pi.c | 2 +- tools/perf/bench/futex-requeue.c | 2 +- tools/perf/bench/futex-wake-parallel.c | 2 +- tools/perf/bench/futex-wake.c | 2 +- tools/perf/builtin-c2c.c | 6 +- tools/perf/builtin-ftrace.c | 2 +- tools/perf/builtin-record.c | 4 +- tools/perf/builtin-stat.c | 31 +------ tools/perf/tests/bitmap.c | 13 +-- tools/perf/tests/code-reading.c | 2 +- tools/perf/tests/cpumap.c | 2 +- tools/perf/tests/keep-tracking.c | 2 +- tools/perf/tests/mmap-basic.c | 2 +- tools/perf/tests/openat-syscall-all-cpus.c | 2 +- tools/perf/tests/perf-time-to-tsc.c | 2 +- tools/perf/tests/sw-clock.c | 2 +- tools/perf/tests/switch-tracking.c | 2 +- tools/perf/tests/task-exit.c | 2 +- tools/perf/tests/topology.c | 48 +++++----- tools/perf/util/auxtrace.c | 4 +- tools/perf/util/bpf_counter.c | 2 +- tools/perf/util/bpf_kwork.c | 16 ++-- tools/perf/util/bpf_kwork_top.c | 12 +-- tools/perf/util/cpumap.c | 14 ++- tools/perf/util/cputopo.c | 2 +- tools/perf/util/evlist.c | 4 +- tools/perf/util/evsel.c | 2 +- tools/perf/util/perf_api_probe.c | 4 +- tools/perf/util/record.c | 4 +- .../scripting-engines/trace-event-python.c | 12 ++- tools/perf/util/session.c | 5 +- tools/perf/util/stat.c | 2 +- tools/perf/util/svghelper.c | 20 ++-- tools/perf/util/top.c | 9 +- 53 files changed, 296 insertions(+), 254 deletions(-)
Comments
On Tue, Nov 28, 2023 at 10:02 PM Ian Rogers <irogers@google.com> wrote: > > Rename and clean up the use of libperf CPU map functions particularly > focussing on perf_cpu_map__empty that may return true for maps > containing CPUs but also with an "any CPU"/dummy value. > > perf_cpu_map__nr is also troubling in that iterating an empty CPU map > will yield the "any CPU"/dummy value. Reduce the appearance of some > calls to this by using the perf_cpu_map__for_each_cpu macro. > > Ian Rogers (14): > libperf cpumap: Rename perf_cpu_map__dummy_new > libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new > libperf cpumap: Rename perf_cpu_map__empty > libperf cpumap: Replace usage of perf_cpu_map__new(NULL) > libperf cpumap: Add for_each_cpu that skips the "any CPU" case > libperf cpumap: Add any, empty and min helpers > perf arm-spe/cs-etm: Directly iterate CPU maps > perf intel-pt/intel-bts: Switch perf_cpu_map__has_any_cpu_or_is_empty > use > perf cpumap: Clean up use of perf_cpu_map__has_any_cpu_or_is_empty > perf top: Avoid repeated function calls > perf arm64 header: Remove unnecessary CPU map get and put > perf stat: Remove duplicate cpus_map_matched function > perf cpumap: Use perf_cpu_map__for_each_cpu when possible > libperf cpumap: Document perf_cpu_map__nr's behavior Ping. Thanks, Ian > .../perf/Documentation/examples/sampling.c | 2 +- > .../perf/Documentation/libperf-sampling.txt | 2 +- > tools/lib/perf/Documentation/libperf.txt | 4 +- > tools/lib/perf/cpumap.c | 92 +++++++++++++------ > tools/lib/perf/evlist.c | 6 +- > tools/lib/perf/evsel.c | 2 +- > tools/lib/perf/include/perf/cpumap.h | 56 ++++++++++- > tools/lib/perf/libperf.map | 10 +- > tools/lib/perf/tests/test-cpumap.c | 4 +- > tools/lib/perf/tests/test-evlist.c | 6 +- > tools/lib/perf/tests/test-evsel.c | 2 +- > tools/perf/arch/arm/util/cs-etm.c | 83 +++++++---------- > tools/perf/arch/arm64/util/arm-spe.c | 4 +- > tools/perf/arch/arm64/util/header.c | 15 +-- > tools/perf/arch/x86/util/intel-bts.c | 4 +- > tools/perf/arch/x86/util/intel-pt.c | 10 +- > tools/perf/bench/epoll-ctl.c | 2 +- > tools/perf/bench/epoll-wait.c | 2 +- > tools/perf/bench/futex-hash.c | 2 +- > tools/perf/bench/futex-lock-pi.c | 2 +- > tools/perf/bench/futex-requeue.c | 2 +- > tools/perf/bench/futex-wake-parallel.c | 2 +- > tools/perf/bench/futex-wake.c | 2 +- > tools/perf/builtin-c2c.c | 6 +- > tools/perf/builtin-ftrace.c | 2 +- > tools/perf/builtin-record.c | 4 +- > tools/perf/builtin-stat.c | 31 +------ > tools/perf/tests/bitmap.c | 13 +-- > tools/perf/tests/code-reading.c | 2 +- > tools/perf/tests/cpumap.c | 2 +- > tools/perf/tests/keep-tracking.c | 2 +- > tools/perf/tests/mmap-basic.c | 2 +- > tools/perf/tests/openat-syscall-all-cpus.c | 2 +- > tools/perf/tests/perf-time-to-tsc.c | 2 +- > tools/perf/tests/sw-clock.c | 2 +- > tools/perf/tests/switch-tracking.c | 2 +- > tools/perf/tests/task-exit.c | 2 +- > tools/perf/tests/topology.c | 48 +++++----- > tools/perf/util/auxtrace.c | 4 +- > tools/perf/util/bpf_counter.c | 2 +- > tools/perf/util/bpf_kwork.c | 16 ++-- > tools/perf/util/bpf_kwork_top.c | 12 +-- > tools/perf/util/cpumap.c | 14 ++- > tools/perf/util/cputopo.c | 2 +- > tools/perf/util/evlist.c | 4 +- > tools/perf/util/evsel.c | 2 +- > tools/perf/util/perf_api_probe.c | 4 +- > tools/perf/util/record.c | 4 +- > .../scripting-engines/trace-event-python.c | 12 ++- > tools/perf/util/session.c | 5 +- > tools/perf/util/stat.c | 2 +- > tools/perf/util/svghelper.c | 20 ++-- > tools/perf/util/top.c | 9 +- > 53 files changed, 296 insertions(+), 254 deletions(-) > > -- > 2.43.0.rc1.413.gea7ed67945-goog >
Em Tue, Nov 28, 2023 at 10:01:57PM -0800, Ian Rogers escreveu: > Rename and clean up the use of libperf CPU map functions particularly > focussing on perf_cpu_map__empty that may return true for maps > containing CPUs but also with an "any CPU"/dummy value. > > perf_cpu_map__nr is also troubling in that iterating an empty CPU map > will yield the "any CPU"/dummy value. Reduce the appearance of some > calls to this by using the perf_cpu_map__for_each_cpu macro. > > Ian Rogers (14): > libperf cpumap: Rename perf_cpu_map__dummy_new > libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new > libperf cpumap: Rename perf_cpu_map__empty > libperf cpumap: Replace usage of perf_cpu_map__new(NULL) > libperf cpumap: Add for_each_cpu that skips the "any CPU" case Applied 1-6, with James Reviewed-by tags, would be good to have Adrian check the PT and BTS parts, testing the end result if he things its all ok. - Arnaldo
On 12/12/23 19:59, Arnaldo Carvalho de Melo wrote: > Em Tue, Nov 28, 2023 at 10:01:57PM -0800, Ian Rogers escreveu: >> Rename and clean up the use of libperf CPU map functions particularly >> focussing on perf_cpu_map__empty that may return true for maps >> containing CPUs but also with an "any CPU"/dummy value. >> >> perf_cpu_map__nr is also troubling in that iterating an empty CPU map >> will yield the "any CPU"/dummy value. Reduce the appearance of some >> calls to this by using the perf_cpu_map__for_each_cpu macro. >> >> Ian Rogers (14): >> libperf cpumap: Rename perf_cpu_map__dummy_new >> libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new >> libperf cpumap: Rename perf_cpu_map__empty >> libperf cpumap: Replace usage of perf_cpu_map__new(NULL) >> libperf cpumap: Add for_each_cpu that skips the "any CPU" case > > Applied 1-6, with James Reviewed-by tags, would be good to have Adrian > check the PT and BTS parts, testing the end result if he things its all > ok. > Changing the same lines of code twice in the same patch set is not really kernel style. Some of the churn could be reduced by applying and rebasing on the patch below. Ideally the patches should be reordered so that the lines only change once i.e. perf_cpu_map__empty -> <replacement> instead of perf_cpu_map__empty -> <rename> -> <replacement> If that is too much trouble, please accept my ack instead: Acked-by: Adrian Hunter <adrian.hunter@intel.com> From: Adrian Hunter <adrian.hunter@intel.com> Factor out perf_cpu_map__empty() use to reduce the occurrences and make the code more readable. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/arch/x86/util/intel-bts.c | 11 ++++++++--- tools/perf/arch/x86/util/intel-pt.c | 21 ++++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c index d2c8cac11470..cebe994eb9db 100644 --- a/tools/perf/arch/x86/util/intel-bts.c +++ b/tools/perf/arch/x86/util/intel-bts.c @@ -59,6 +59,11 @@ intel_bts_info_priv_size(struct auxtrace_record *itr __maybe_unused, return INTEL_BTS_AUXTRACE_PRIV_SIZE; } +static bool intel_bts_per_cpu(struct evlist *evlist) +{ + return !perf_cpu_map__empty(evlist->core.user_requested_cpus); +} + static int intel_bts_info_fill(struct auxtrace_record *itr, struct perf_session *session, struct perf_record_auxtrace_info *auxtrace_info, @@ -109,8 +114,8 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, struct intel_bts_recording *btsr = container_of(itr, struct intel_bts_recording, itr); struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu; + bool per_cpu_mmaps = intel_bts_per_cpu(evlist); struct evsel *evsel, *intel_bts_evsel = NULL; - const struct perf_cpu_map *cpus = evlist->core.user_requested_cpus; bool privileged = perf_event_paranoid_check(-1); if (opts->auxtrace_sample_mode) { @@ -143,7 +148,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, if (!opts->full_auxtrace) return 0; - if (opts->full_auxtrace && !perf_cpu_map__empty(cpus)) { + if (opts->full_auxtrace && per_cpu_mmaps) { pr_err(INTEL_BTS_PMU_NAME " does not support per-cpu recording\n"); return -EINVAL; } @@ -224,7 +229,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, * In the case of per-cpu mmaps, we need the CPU on the * AUX event. */ - if (!perf_cpu_map__empty(cpus)) + if (per_cpu_mmaps) evsel__set_sample_bit(intel_bts_evsel, CPU); } diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c index fa0c718b9e72..0ff9147c75da 100644 --- a/tools/perf/arch/x86/util/intel-pt.c +++ b/tools/perf/arch/x86/util/intel-pt.c @@ -312,6 +312,11 @@ static void intel_pt_tsc_ctc_ratio(u32 *n, u32 *d) *d = eax; } +static bool intel_pt_per_cpu(struct evlist *evlist) +{ + return !perf_cpu_map__empty(evlist->core.user_requested_cpus); +} + static int intel_pt_info_fill(struct auxtrace_record *itr, struct perf_session *session, struct perf_record_auxtrace_info *auxtrace_info, @@ -322,7 +327,8 @@ static int intel_pt_info_fill(struct auxtrace_record *itr, struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu; struct perf_event_mmap_page *pc; struct perf_tsc_conversion tc = { .time_mult = 0, }; - bool cap_user_time_zero = false, per_cpu_mmaps; + bool per_cpu_mmaps = intel_pt_per_cpu(session->evlist); + bool cap_user_time_zero = false; u64 tsc_bit, mtc_bit, mtc_freq_bits, cyc_bit, noretcomp_bit; u32 tsc_ctc_ratio_n, tsc_ctc_ratio_d; unsigned long max_non_turbo_ratio; @@ -369,8 +375,6 @@ static int intel_pt_info_fill(struct auxtrace_record *itr, ui__warning("Intel Processor Trace: TSC not available\n"); } - per_cpu_mmaps = !perf_cpu_map__empty(session->evlist->core.user_requested_cpus); - auxtrace_info->type = PERF_AUXTRACE_INTEL_PT; auxtrace_info->priv[INTEL_PT_PMU_TYPE] = intel_pt_pmu->type; auxtrace_info->priv[INTEL_PT_TIME_SHIFT] = tc.time_shift; @@ -604,8 +608,8 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu; bool have_timing_info, need_immediate = false; struct evsel *evsel, *intel_pt_evsel = NULL; - const struct perf_cpu_map *cpus = evlist->core.user_requested_cpus; bool privileged = perf_event_paranoid_check(-1); + bool per_cpu_mmaps = intel_pt_per_cpu(evlist); u64 tsc_bit; int err; @@ -774,8 +778,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, * Per-cpu recording needs sched_switch events to distinguish different * threads. */ - if (have_timing_info && !perf_cpu_map__empty(cpus) && - !record_opts__no_switch_events(opts)) { + if (have_timing_info && per_cpu_mmaps && !record_opts__no_switch_events(opts)) { if (perf_can_record_switch_events()) { bool cpu_wide = !target__none(&opts->target) && !target__has_task(&opts->target); @@ -832,7 +835,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, * In the case of per-cpu mmaps, we need the CPU on the * AUX event. */ - if (!perf_cpu_map__empty(cpus)) + if (per_cpu_mmaps) evsel__set_sample_bit(intel_pt_evsel, CPU); } @@ -858,7 +861,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, tracking_evsel->immediate = true; /* In per-cpu case, always need the time of mmap events etc */ - if (!perf_cpu_map__empty(cpus)) { + if (per_cpu_mmaps) { evsel__set_sample_bit(tracking_evsel, TIME); /* And the CPU for switch events */ evsel__set_sample_bit(tracking_evsel, CPU); @@ -870,7 +873,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, * Warn the user when we do not have enough information to decode i.e. * per-cpu with no sched_switch (except workload-only). */ - if (!ptr->have_sched_switch && !perf_cpu_map__empty(cpus) && + if (!ptr->have_sched_switch && per_cpu_mmaps && !target__none(&opts->target) && !intel_pt_evsel->core.attr.exclude_user) ui__warning("Intel Processor Trace decoding will not be possible except for kernel tracing!\n");
On Tue, Nov 28, 2023 at 10:02 PM Ian Rogers <irogers@google.com> wrote: > > Rename and clean up the use of libperf CPU map functions particularly > focussing on perf_cpu_map__empty that may return true for maps > containing CPUs but also with an "any CPU"/dummy value. > > perf_cpu_map__nr is also troubling in that iterating an empty CPU map > will yield the "any CPU"/dummy value. Reduce the appearance of some > calls to this by using the perf_cpu_map__for_each_cpu macro. > > Ian Rogers (14): > libperf cpumap: Rename perf_cpu_map__dummy_new > libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new > libperf cpumap: Rename perf_cpu_map__empty > libperf cpumap: Replace usage of perf_cpu_map__new(NULL) > libperf cpumap: Add for_each_cpu that skips the "any CPU" case > libperf cpumap: Add any, empty and min helpers > perf arm-spe/cs-etm: Directly iterate CPU maps > perf intel-pt/intel-bts: Switch perf_cpu_map__has_any_cpu_or_is_empty > use > perf cpumap: Clean up use of perf_cpu_map__has_any_cpu_or_is_empty > perf top: Avoid repeated function calls > perf arm64 header: Remove unnecessary CPU map get and put > perf stat: Remove duplicate cpus_map_matched function > perf cpumap: Use perf_cpu_map__for_each_cpu when possible > libperf cpumap: Document perf_cpu_map__nr's behavior Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks, Namhyung
Em Wed, Dec 13, 2023 at 02:48:15PM +0200, Adrian Hunter escreveu: > On 12/12/23 19:59, Arnaldo Carvalho de Melo wrote: > > Em Tue, Nov 28, 2023 at 10:01:57PM -0800, Ian Rogers escreveu: > >> Rename and clean up the use of libperf CPU map functions particularly > >> focussing on perf_cpu_map__empty that may return true for maps > >> containing CPUs but also with an "any CPU"/dummy value. > >> > >> perf_cpu_map__nr is also troubling in that iterating an empty CPU map > >> will yield the "any CPU"/dummy value. Reduce the appearance of some > >> calls to this by using the perf_cpu_map__for_each_cpu macro. > >> > >> Ian Rogers (14): > >> libperf cpumap: Rename perf_cpu_map__dummy_new > >> libperf cpumap: Rename and prefer sysfs for perf_cpu_map__default_new > >> libperf cpumap: Rename perf_cpu_map__empty > >> libperf cpumap: Replace usage of perf_cpu_map__new(NULL) > >> libperf cpumap: Add for_each_cpu that skips the "any CPU" case > > > > Applied 1-6, with James Reviewed-by tags, would be good to have Adrian > > check the PT and BTS parts, testing the end result if he things its all > > ok. Ian, 1-6 is in perf-tools-next now, can you please consider Adrian's suggestion to reduce patch size and rebase the remaining patches? - Arnaldo > Changing the same lines of code twice in the same patch set is not > really kernel style. > > Some of the churn could be reduced by applying and rebasing on the > patch below. > > Ideally the patches should be reordered so that the lines only > change once i.e. > > perf_cpu_map__empty -> <replacement> > > instead of > > perf_cpu_map__empty -> <rename> -> <replacement> > > If that is too much trouble, please accept my ack instead: > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> > > From: Adrian Hunter <adrian.hunter@intel.com> > > Factor out perf_cpu_map__empty() use to reduce the occurrences and make > the code more readable. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/arch/x86/util/intel-bts.c | 11 ++++++++--- > tools/perf/arch/x86/util/intel-pt.c | 21 ++++++++++++--------- > 2 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c > index d2c8cac11470..cebe994eb9db 100644 > --- a/tools/perf/arch/x86/util/intel-bts.c > +++ b/tools/perf/arch/x86/util/intel-bts.c > @@ -59,6 +59,11 @@ intel_bts_info_priv_size(struct auxtrace_record *itr __maybe_unused, > return INTEL_BTS_AUXTRACE_PRIV_SIZE; > } > > +static bool intel_bts_per_cpu(struct evlist *evlist) > +{ > + return !perf_cpu_map__empty(evlist->core.user_requested_cpus); > +} > + > static int intel_bts_info_fill(struct auxtrace_record *itr, > struct perf_session *session, > struct perf_record_auxtrace_info *auxtrace_info, > @@ -109,8 +114,8 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, > struct intel_bts_recording *btsr = > container_of(itr, struct intel_bts_recording, itr); > struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu; > + bool per_cpu_mmaps = intel_bts_per_cpu(evlist); > struct evsel *evsel, *intel_bts_evsel = NULL; > - const struct perf_cpu_map *cpus = evlist->core.user_requested_cpus; > bool privileged = perf_event_paranoid_check(-1); > > if (opts->auxtrace_sample_mode) { > @@ -143,7 +148,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, > if (!opts->full_auxtrace) > return 0; > > - if (opts->full_auxtrace && !perf_cpu_map__empty(cpus)) { > + if (opts->full_auxtrace && per_cpu_mmaps) { > pr_err(INTEL_BTS_PMU_NAME " does not support per-cpu recording\n"); > return -EINVAL; > } > @@ -224,7 +229,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, > * In the case of per-cpu mmaps, we need the CPU on the > * AUX event. > */ > - if (!perf_cpu_map__empty(cpus)) > + if (per_cpu_mmaps) > evsel__set_sample_bit(intel_bts_evsel, CPU); > } > > diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c > index fa0c718b9e72..0ff9147c75da 100644 > --- a/tools/perf/arch/x86/util/intel-pt.c > +++ b/tools/perf/arch/x86/util/intel-pt.c > @@ -312,6 +312,11 @@ static void intel_pt_tsc_ctc_ratio(u32 *n, u32 *d) > *d = eax; > } > > +static bool intel_pt_per_cpu(struct evlist *evlist) > +{ > + return !perf_cpu_map__empty(evlist->core.user_requested_cpus); > +} > + > static int intel_pt_info_fill(struct auxtrace_record *itr, > struct perf_session *session, > struct perf_record_auxtrace_info *auxtrace_info, > @@ -322,7 +327,8 @@ static int intel_pt_info_fill(struct auxtrace_record *itr, > struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu; > struct perf_event_mmap_page *pc; > struct perf_tsc_conversion tc = { .time_mult = 0, }; > - bool cap_user_time_zero = false, per_cpu_mmaps; > + bool per_cpu_mmaps = intel_pt_per_cpu(session->evlist); > + bool cap_user_time_zero = false; > u64 tsc_bit, mtc_bit, mtc_freq_bits, cyc_bit, noretcomp_bit; > u32 tsc_ctc_ratio_n, tsc_ctc_ratio_d; > unsigned long max_non_turbo_ratio; > @@ -369,8 +375,6 @@ static int intel_pt_info_fill(struct auxtrace_record *itr, > ui__warning("Intel Processor Trace: TSC not available\n"); > } > > - per_cpu_mmaps = !perf_cpu_map__empty(session->evlist->core.user_requested_cpus); > - > auxtrace_info->type = PERF_AUXTRACE_INTEL_PT; > auxtrace_info->priv[INTEL_PT_PMU_TYPE] = intel_pt_pmu->type; > auxtrace_info->priv[INTEL_PT_TIME_SHIFT] = tc.time_shift; > @@ -604,8 +608,8 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, > struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu; > bool have_timing_info, need_immediate = false; > struct evsel *evsel, *intel_pt_evsel = NULL; > - const struct perf_cpu_map *cpus = evlist->core.user_requested_cpus; > bool privileged = perf_event_paranoid_check(-1); > + bool per_cpu_mmaps = intel_pt_per_cpu(evlist); > u64 tsc_bit; > int err; > > @@ -774,8 +778,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, > * Per-cpu recording needs sched_switch events to distinguish different > * threads. > */ > - if (have_timing_info && !perf_cpu_map__empty(cpus) && > - !record_opts__no_switch_events(opts)) { > + if (have_timing_info && per_cpu_mmaps && !record_opts__no_switch_events(opts)) { > if (perf_can_record_switch_events()) { > bool cpu_wide = !target__none(&opts->target) && > !target__has_task(&opts->target); > @@ -832,7 +835,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, > * In the case of per-cpu mmaps, we need the CPU on the > * AUX event. > */ > - if (!perf_cpu_map__empty(cpus)) > + if (per_cpu_mmaps) > evsel__set_sample_bit(intel_pt_evsel, CPU); > } > > @@ -858,7 +861,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, > tracking_evsel->immediate = true; > > /* In per-cpu case, always need the time of mmap events etc */ > - if (!perf_cpu_map__empty(cpus)) { > + if (per_cpu_mmaps) { > evsel__set_sample_bit(tracking_evsel, TIME); > /* And the CPU for switch events */ > evsel__set_sample_bit(tracking_evsel, CPU); > @@ -870,7 +873,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, > * Warn the user when we do not have enough information to decode i.e. > * per-cpu with no sched_switch (except workload-only). > */ > - if (!ptr->have_sched_switch && !perf_cpu_map__empty(cpus) && > + if (!ptr->have_sched_switch && per_cpu_mmaps && > !target__none(&opts->target) && > !intel_pt_evsel->core.attr.exclude_user) > ui__warning("Intel Processor Trace decoding will not be possible except for kernel tracing!\n"); > -- > 2.34.1 > > >