On Mon, Nov 08, 2021 at 09:53:44AM -0800, Ian Rogers wrote:

On Mon, Nov 8, 2021 at 5:38 AM Jiri Olsa [off-list ref] wrote:

quoted

Moving exclude_GH to perf_evsel struct.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/perf/include/internal/evsel.h | 1 +
 tools/perf/util/evsel.c                 | 2 +-
 tools/perf/util/evsel.h                 | 1 -
 tools/perf/util/parse-events.c          | 4 ++--
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index b96c9f4808ca..7f3ee73c5aa5 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h

@@ -69,6 +69,7 @@ struct perf_evsel {
                const char              *metric_id;
                enum perf_tool_event    tool_event;
                const char              *unit;
+               int                     exclude_GH;

I think it'd be pretty awesome if when these things get moved around
we added a comment here to explain what the variable means. Perhaps
even put an example :-) In this case it could explain why this is an
int and not a bool. This is a while you are here request rather than a
criticism of the changes which I find fine.

ok, I noticed you added several comments like that already,
I'll put some in here in next version

thanks,
jirka

Thanks,
Ian

quoted

        };
 };

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 133b571e1190..802c7de9e8e3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c

@@ -431,7 +431,7 @@ struct evsel *evsel__clone(struct evsel *orig)
        evsel->use_uncore_alias = orig->use_uncore_alias;
        evsel->is_libpfm_event = orig->is_libpfm_event;

-       evsel->exclude_GH = orig->exclude_GH;
+       evsel->core.exclude_GH = orig->core.exclude_GH;
        evsel->sample_read = orig->sample_read;
        evsel->core.auto_merge_stats = orig->core.auto_merge_stats;
        evsel->collect_stat = orig->collect_stat;

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index f6cc1e78903f..6c3634ed169a 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h

@@ -62,7 +62,6 @@ struct evsel {
                double                  scale;
                struct cgroup           *cgrp;
                /* parse modifier helper */
-               int                     exclude_GH;
                int                     sample_read;
                bool                    snapshot;
                bool                    per_pkg;

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1878ea1310d3..89787ebc23ef 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

@@ -1918,7 +1918,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
        int exclusive = evsel ? evsel->core.attr.exclusive : 0;

        int exclude = eu | ek | eh;
-       int exclude_GH = evsel ? evsel->exclude_GH : 0;
+       int exclude_GH = evsel ? evsel->core.exclude_GH : 0;
        int weak = 0;
        int bpf_counter = 0;

@@ -2049,7 +2049,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
                evsel->core.attr.exclude_host   = mod.eH;
                evsel->core.attr.exclude_guest  = mod.eG;
                evsel->core.attr.exclude_idle   = mod.eI;
-               evsel->exclude_GH          = mod.exclude_GH;
+               evsel->core.exclude_GH          = mod.exclude_GH;
                evsel->sample_read         = mod.sample_read;
                evsel->precise_max         = mod.precise_max;
                evsel->weak_group          = mod.weak;
--

2.31.1