On Wed, May 15, 2024 at 9:20 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> Give the term types their own enum so that additional terms can be
> added that don't correspond to a PERF_SAMPLE_xx flag. The term values
> are numerically ascending rather than bit field positions, this means
> they need translating to a PERF_SAMPLE_xx bit field in certain places
> and they are more densely encoded.
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
[SNIP]
> diff --git a/tools/perf/util/bpf_skel/sample-filter.h b/tools/perf/util/bpf_skel/sample-filter.h
> index 2e96e1ab084a..161d5ff49cb6 100644
> --- a/tools/perf/util/bpf_skel/sample-filter.h
> +++ b/tools/perf/util/bpf_skel/sample-filter.h
> @@ -16,12 +16,32 @@ enum perf_bpf_filter_op {
> PBF_OP_GROUP_END,
> };
>
> +enum perf_bpf_filter_term {
> + /* No term is in use. */
> + PBF_TERM_NONE,
> + /* Terms that correspond to PERF_SAMPLE_xx values. */
> + PBF_TERM_IP,
> + PBF_TERM_ID,
> + PBF_TERM_TID,
> + PBF_TERM_CPU,
> + PBF_TERM_TIME,
> + PBF_TERM_ADDR,
> + PBF_TERM_PERIOD,
> + PBF_TERM_TRANSACTION,
> + PBF_TERM_WEIGHT,
> + PBF_TERM_PHYS_ADDR,
> + PBF_TERM_CODE_PAGE_SIZE,
> + PBF_TERM_DATA_PAGE_SIZE,
> + PBF_TERM_WEIGHT_STRUCT,
> + PBF_TERM_DATA_SRC,
> +};
> +
> /* BPF map entry for filtering */
> struct perf_bpf_filter_entry {
> enum perf_bpf_filter_op op;
> __u32 part; /* sub-sample type info when it has multiple values */
> - __u64 flags; /* perf sample type flags */
> + enum perf_bpf_filter_term term;
> __u64 value;
> };
>
> -#endif /* PERF_UTIL_BPF_SKEL_SAMPLE_FILTER_H */
> \ No newline at end of file
> +#endif /* PERF_UTIL_BPF_SKEL_SAMPLE_FILTER_H */
> diff --git a/tools/perf/util/bpf_skel/sample_filter.bpf.c b/tools/perf/util/bpf_skel/sample_filter.bpf.c
> index fb94f5280626..8666c85e9333 100644
> --- a/tools/perf/util/bpf_skel/sample_filter.bpf.c
> +++ b/tools/perf/util/bpf_skel/sample_filter.bpf.c
> @@ -48,31 +48,50 @@ static inline __u64 perf_get_sample(struct bpf_perf_event_data_kern *kctx,
> {
> struct perf_sample_data___new *data = (void *)kctx->data;
>
> - if (!bpf_core_field_exists(data->sample_flags) ||
> - (data->sample_flags & entry->flags) == 0)
> + if (!bpf_core_field_exists(data->sample_flags))
> return 0;
>
> - switch (entry->flags) {
> - case PERF_SAMPLE_IP:
> + switch (entry->term) {
> + case PBF_TERM_NONE:
> + return 0;
> + case PBF_TERM_IP:
> + if ((data->sample_flags & PERF_SAMPLE_IP) == 0)
> + return 0;
Can we check this in a single place like in the original code
instead of doing it in every case? I think we can keep the
entry->flags and check it only if it's non zero. Then uid and
gid will have 0 to skip.
Thanks,
Namhyung
> return kctx->data->ip;
> - case PERF_SAMPLE_ID:
> + case PBF_TERM_ID:
> + if ((data->sample_flags & PERF_SAMPLE_ID) == 0)
> + return 0;
> return kctx->data->id;
> - case PERF_SAMPLE_TID:
> + case PBF_TERM_TID:
> + if ((data->sample_flags & PERF_SAMPLE_TID) == 0)
> + return 0;
> if (entry->part)
> return kctx->data->tid_entry.pid;
> else
> return kctx->data->tid_entry.tid;
> - case PERF_SAMPLE_CPU:
> + case PBF_TERM_CPU:
> + if ((data->sample_flags & PERF_SAMPLE_CPU) == 0)
> + return 0;
> return kctx->data->cpu_entry.cpu;
> - case PERF_SAMPLE_TIME:
> + case PBF_TERM_TIME:
> + if ((data->sample_flags & PERF_SAMPLE_TIME) == 0)
> + return 0;
> return kctx->data->time;
> - case PERF_SAMPLE_ADDR:
> + case PBF_TERM_ADDR:
> + if ((data->sample_flags & PERF_SAMPLE_ADDR) == 0)
> + return 0;
> return kctx->data->addr;
> - case PERF_SAMPLE_PERIOD:
> + case PBF_TERM_PERIOD:
> + if ((data->sample_flags & PERF_SAMPLE_PERIOD) == 0)
> + return 0;
> return kctx->data->period;
> - case PERF_SAMPLE_TRANSACTION:
> + case PBF_TERM_TRANSACTION:
> + if ((data->sample_flags & PERF_SAMPLE_TRANSACTION) == 0)
> + return 0;
> return kctx->data->txn;
> - case PERF_SAMPLE_WEIGHT_STRUCT:
> + case PBF_TERM_WEIGHT_STRUCT:
> + if ((data->sample_flags & PERF_SAMPLE_WEIGHT_STRUCT) == 0)
> + return 0;
> if (entry->part == 1)
> return kctx->data->weight.var1_dw;
> if (entry->part == 2)
> @@ -80,15 +99,25 @@ static inline __u64 perf_get_sample(struct bpf_perf_event_data_kern *kctx,
> if (entry->part == 3)
> return kctx->data->weight.var3_w;
> /* fall through */
> - case PERF_SAMPLE_WEIGHT:
> + case PBF_TERM_WEIGHT:
> + if ((data->sample_flags & PERF_SAMPLE_WEIGHT) == 0)
> + return 0;
> return kctx->data->weight.full;
> - case PERF_SAMPLE_PHYS_ADDR:
> + case PBF_TERM_PHYS_ADDR:
> + if ((data->sample_flags & PERF_SAMPLE_PHYS_ADDR) == 0)
> + return 0;
> return kctx->data->phys_addr;
> - case PERF_SAMPLE_CODE_PAGE_SIZE:
> + case PBF_TERM_CODE_PAGE_SIZE:
> + if ((data->sample_flags & PERF_SAMPLE_CODE_PAGE_SIZE) == 0)
> + return 0;
> return kctx->data->code_page_size;
> - case PERF_SAMPLE_DATA_PAGE_SIZE:
> + case PBF_TERM_DATA_PAGE_SIZE:
> + if ((data->sample_flags & PERF_SAMPLE_DATA_PAGE_SIZE) == 0)
> + return 0;
> return kctx->data->data_page_size;
> - case PERF_SAMPLE_DATA_SRC:
> + case PBF_TERM_DATA_SRC:
> + if ((data->sample_flags & PERF_SAMPLE_DATA_SRC) == 0)
> + return 0;
> if (entry->part == 1)
> return kctx->data->data_src.mem_op;
> if (entry->part == 2)
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
