From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: kan.liang@linux.intel.com
Cc: mingo@kernel.org, linux-kernel@vger.kernel.org,
peterz@infradead.org, eranian@google.com, namhyung@kernel.org,
jolsa@redhat.com, ak@linux.intel.com, yao.jin@linux.intel.com,
maddy@linux.vnet.ibm.com
Subject: Re: [PATCH 5/9] perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT
Date: Wed, 3 Feb 2021 17:31:05 -0300 [thread overview]
Message-ID: <20210203203105.GJ854763@kernel.org> (raw)
In-Reply-To: <1612296553-21962-6-git-send-email-kan.liang@linux.intel.com>
Em Tue, Feb 02, 2021 at 12:09:09PM -0800, kan.liang@linux.intel.com escreveu:
> From: Kan Liang <kan.liang@linux.intel.com>
>
> The new sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
> PERF_SAMPLE_WEIGHT sample type. Users can apply either the
> PERF_SAMPLE_WEIGHT sample type or the PERF_SAMPLE_WEIGHT_STRUCT sample
> type to retrieve the sample weight, but they cannot apply both sample
> types simultaneously. The new sample type shares the same space as the
> PERF_SAMPLE_WEIGHT sample type. The lower 32 bits are exactly the same
> for both sample type. The higher 32 bits may be different for different
> architecture.
>
> Add arch specific arch_evsel__set_sample_weight() to set the new sample
> type for X86. Only store the lower 32 bits for the sample->weight if the
> new sample type is applied. In practice, no memory access could last
> than 4G cycles. No data will be lost.
>
> If the kernel doesn't support the new sample type. Fall back to the
> PERF_SAMPLE_WEIGHT sample type.
>
> There is no impact for other architectures.
>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
> tools/perf/arch/x86/util/Build | 1 +
> tools/perf/arch/x86/util/evsel.c | 8 ++++++++
> tools/perf/util/evsel.c | 28 ++++++++++++++++++++++++----
> tools/perf/util/evsel.h | 3 +++
> tools/perf/util/intel-pt.c | 22 +++++++++++++++++++---
> tools/perf/util/perf_event_attr_fprintf.c | 2 +-
> tools/perf/util/session.c | 2 +-
> tools/perf/util/synthetic-events.c | 6 ++++--
> 8 files changed, 61 insertions(+), 11 deletions(-)
> create mode 100644 tools/perf/arch/x86/util/evsel.c
>
> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
> index d73f548..18848b3 100644
> --- a/tools/perf/arch/x86/util/Build
> +++ b/tools/perf/arch/x86/util/Build
> @@ -7,6 +7,7 @@ perf-y += topdown.o
> perf-y += machine.o
> perf-y += event.o
> perf-y += mem-events.o
> +perf-y += evsel.o
>
> perf-$(CONFIG_DWARF) += dwarf-regs.o
> perf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
> new file mode 100644
> index 0000000..2f733cd
> --- /dev/null
> +++ b/tools/perf/arch/x86/util/evsel.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
> +#include "util/evsel.h"
> +
> +void arch_evsel__set_sample_weight(struct evsel *evsel)
> +{
> + evsel__set_sample_bit(evsel, WEIGHT_STRUCT);
> +}
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index c48f6de..0a2a307 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1012,6 +1012,11 @@ struct evsel_config_term *__evsel__get_config_term(struct evsel *evsel, enum evs
> return found_term;
> }
>
> +void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
> +{
> + evsel__set_sample_bit(evsel, WEIGHT);
> +}
> +
> /*
> * The enable_on_exec/disabled value strategy:
> *
> @@ -1166,7 +1171,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
> }
>
> if (opts->sample_weight)
> - evsel__set_sample_bit(evsel, WEIGHT);
> + arch_evsel__set_sample_weight(evsel);
>
> attr->task = track;
> attr->mmap = track;
> @@ -1735,6 +1740,10 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> }
>
> fallback_missing_features:
> + if (perf_missing_features.weight_struct) {
> + evsel__set_sample_bit(evsel, WEIGHT);
> + evsel__reset_sample_bit(evsel, WEIGHT_STRUCT);
> + }
> if (perf_missing_features.clockid_wrong)
> evsel->core.attr.clockid = CLOCK_MONOTONIC; /* should always work */
> if (perf_missing_features.clockid) {
> @@ -1873,7 +1882,12 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> * Must probe features in the order they were added to the
> * perf_event_attr interface.
> */
> - if (!perf_missing_features.data_page_size &&
> + if (!perf_missing_features.weight_struct &&
> + (evsel->core.attr.sample_type & PERF_SAMPLE_WEIGHT_STRUCT)) {
> + perf_missing_features.weight_struct = true;
> + pr_debug2("switching off weight struct support\n");
> + goto fallback_missing_features;
> + } else if (!perf_missing_features.data_page_size &&
> (evsel->core.attr.sample_type & PERF_SAMPLE_DATA_PAGE_SIZE)) {
> perf_missing_features.data_page_size = true;
> pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support, bailing out\n");
> @@ -2316,9 +2330,15 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> }
> }
>
> - if (type & PERF_SAMPLE_WEIGHT) {
> + if (type & PERF_SAMPLE_WEIGHT_TYPE) {
> + union perf_sample_weight weight;
> +
> OVERFLOW_CHECK_u64(array);
> - data->weight = *array;
> + weight.full = *array;
> + if (type & PERF_SAMPLE_WEIGHT)
> + data->weight = weight.full;
> + else
> + data->weight = weight.var1_dw;
> array++;
> }
>
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index cd1d8dd..5c161a2 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -145,6 +145,7 @@ struct perf_missing_features {
> bool branch_hw_idx;
> bool cgroup;
> bool data_page_size;
> + bool weight_struct;
> };
>
> extern struct perf_missing_features perf_missing_features;
> @@ -239,6 +240,8 @@ void __evsel__reset_sample_bit(struct evsel *evsel, enum perf_event_sample_forma
>
> void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);
>
> +void arch_evsel__set_sample_weight(struct evsel *evsel);
> +
> int evsel__set_filter(struct evsel *evsel, const char *filter);
> int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
> int evsel__append_addr_filter(struct evsel *evsel, const char *filter);
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index 60214de..a929f6d 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -1853,13 +1853,29 @@ static int intel_pt_synth_pebs_sample(struct intel_pt_queue *ptq)
> if (sample_type & PERF_SAMPLE_ADDR && items->has_mem_access_address)
> sample.addr = items->mem_access_address;
>
> - if (sample_type & PERF_SAMPLE_WEIGHT) {
> + if (sample_type & PERF_SAMPLE_WEIGHT_TYPE) {
> /*
> * Refer kernel's setup_pebs_adaptive_sample_data() and
> * intel_hsw_weight().
> */
> - if (items->has_mem_access_latency)
> - sample.weight = items->mem_access_latency;
> + if (items->has_mem_access_latency) {
> + u64 weight = items->mem_access_latency >> 32;
> +
> + /*
> + * Starts from SPR, the mem access latency field
> + * contains both cache latency [47:32] and instruction
> + * latency [15:0]. The cache latency is the same as the
> + * mem access latency on previous platforms.
> + *
> + * In practice, no memory access could last than 4G
> + * cycles. Use latency >> 32 to distinguish the
> + * different format of the mem access latency field.
> + */
> + if (weight > 0)
> + sample.weight = weight & 0xffff;
> + else
> + sample.weight = items->mem_access_latency;
> + }
> if (!sample.weight && items->has_tsx_aux_info) {
> /* Cycles last block */
> sample.weight = (u32)items->tsx_aux_info;
> diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
> index fb0bb66..e972e63 100644
> --- a/tools/perf/util/perf_event_attr_fprintf.c
> +++ b/tools/perf/util/perf_event_attr_fprintf.c
> @@ -35,7 +35,7 @@ static void __p_sample_type(char *buf, size_t size, u64 value)
> bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
> bit_name(IDENTIFIER), bit_name(REGS_INTR), bit_name(DATA_SRC),
> bit_name(WEIGHT), bit_name(PHYS_ADDR), bit_name(AUX),
> - bit_name(CGROUP), bit_name(DATA_PAGE_SIZE),
> + bit_name(CGROUP), bit_name(DATA_PAGE_SIZE), bit_name(WEIGHT_STRUCT),
I have CODE_PAGE_SIZE in my perf/core branch, was this somehow removed?
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c1de7f3d84ca324c7cda85c3ce27b11741af2124
I see, you did this patchkit on top of upstream, that has just
DATA_PAGE_SIZE, while my perf/core branch has CODE_PAGE_SIZE. I'm
adjusting it, please next time do tooling development on acme/perf/core.
- Arnaldo
> { .name = NULL, }
> };
> #undef bit_name
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 25adbcc..a35d8c2 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1297,7 +1297,7 @@ static void dump_sample(struct evsel *evsel, union perf_event *event,
> if (sample_type & PERF_SAMPLE_STACK_USER)
> stack_user__printf(&sample->user_stack);
>
> - if (sample_type & PERF_SAMPLE_WEIGHT)
> + if (sample_type & PERF_SAMPLE_WEIGHT_TYPE)
> printf("... weight: %" PRIu64 "\n", sample->weight);
>
> if (sample_type & PERF_SAMPLE_DATA_SRC)
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 2947e3f..bc16268 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1384,7 +1384,7 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
> }
> }
>
> - if (type & PERF_SAMPLE_WEIGHT)
> + if (type & PERF_SAMPLE_WEIGHT_TYPE)
> result += sizeof(u64);
>
> if (type & PERF_SAMPLE_DATA_SRC)
> @@ -1555,8 +1555,10 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_fo
> }
> }
>
> - if (type & PERF_SAMPLE_WEIGHT) {
> + if (type & PERF_SAMPLE_WEIGHT_TYPE) {
> *array = sample->weight;
> + if (type & PERF_SAMPLE_WEIGHT_STRUCT)
> + *array &= 0xffffffff;
> array++;
> }
>
> --
> 2.7.4
>
--
- Arnaldo
next prev parent reply other threads:[~2021-02-03 20:32 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-02 20:09 [PATCH 0/9] perf core PMU support for Sapphire Rapids (User tools) kan.liang
2021-02-02 20:09 ` [PATCH 1/9] tools headers uapi: Update tools's copy of linux/perf_event.h kan.liang
2021-02-02 20:09 ` [PATCH 2/9] perf tools: Support the auxiliary event kan.liang
2021-02-03 20:02 ` Arnaldo Carvalho de Melo
2021-02-03 21:20 ` Liang, Kan
2021-02-03 21:30 ` Arnaldo Carvalho de Melo
2021-02-05 10:52 ` Namhyung Kim
2021-02-05 14:13 ` Liang, Kan
2021-02-05 15:26 ` Arnaldo Carvalho de Melo
2021-02-05 15:45 ` Liang, Kan
2021-02-02 20:09 ` [PATCH 3/9] perf tools: Support data block and addr block kan.liang
2021-02-05 11:02 ` Namhyung Kim
2021-02-05 14:17 ` Liang, Kan
2021-02-02 20:09 ` [PATCH 4/9] perf c2c: " kan.liang
2021-02-03 20:21 ` Arnaldo Carvalho de Melo
2021-02-02 20:09 ` [PATCH 5/9] perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT kan.liang
2021-02-03 20:31 ` Arnaldo Carvalho de Melo [this message]
2021-02-03 21:19 ` Liang, Kan
2021-02-03 21:29 ` Arnaldo Carvalho de Melo
2021-02-02 20:09 ` [PATCH 6/9] perf report: Support instruction latency kan.liang
2021-02-03 20:43 ` Arnaldo Carvalho de Melo
2021-02-04 13:11 ` Athira Rajeev
2021-02-04 15:19 ` Liang, Kan
2021-02-05 12:55 ` Athira Rajeev
2021-02-05 14:51 ` Liang, Kan
2021-02-07 16:45 ` Athira Rajeev
2021-02-05 11:08 ` Namhyung Kim
2021-02-05 14:38 ` Liang, Kan
2021-02-06 8:09 ` Namhyung Kim
2021-02-08 13:50 ` Liang, Kan
2021-02-02 20:09 ` [PATCH 7/9] perf test: Support PERF_SAMPLE_WEIGHT_STRUCT kan.liang
2021-02-03 20:44 ` Arnaldo Carvalho de Melo
2021-02-02 20:09 ` [PATCH 8/9] perf stat: Support L2 Topdown events kan.liang
2021-02-02 20:09 ` [PATCH 9/9] perf, tools: Update topdown documentation for Sapphire Rapids kan.liang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210203203105.GJ854763@kernel.org \
--to=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=eranian@google.com \
--cc=jolsa@redhat.com \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maddy@linux.vnet.ibm.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=yao.jin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).