Re: [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel
From: Arnaldo Carvalho de Melo
Date: Tue Dec 15 2020 - 10:17:50 EST
Em Thu, Dec 10, 2020 at 09:43:28PM +0100, Jiri Olsa escreveu:
> Adding interface to enable/disable single event in the
> evlist based on its name. It will be used later in new
> control enable/disable interface.
>
> Keeping the evlist::enabled true when one or more events
> are enabled so the toggle can work properly and toggle
> evlist to disabled state.
Thanks, applied.
- Arnaldo
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> Acked-by: Alexei Budankov <abudankov@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/util/evlist.c | 69 ++++++++++++++++++++++++++++++++++++++--
> tools/perf/util/evlist.h | 2 ++
> 2 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 493819173a8e..70aff26612a9 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -370,7 +370,30 @@ bool evsel__cpu_iter_skip(struct evsel *ev, int cpu)
> return true;
> }
>
> -void evlist__disable(struct evlist *evlist)
> +static int evsel__strcmp(struct evsel *pos, char *evsel_name)
> +{
> + if (!evsel_name)
> + return 0;
> + if (evsel__is_dummy_event(pos))
> + return 1;
> + return strcmp(pos->name, evsel_name);
> +}
> +
> +static int evlist__is_enabled(struct evlist *evlist)
> +{
> + struct evsel *pos;
> +
> + evlist__for_each_entry(evlist, pos) {
> + if (!evsel__is_group_leader(pos) || !pos->core.fd)
> + continue;
> + /* If at least one event is enabled, evlist is enabled. */
> + if (!pos->disabled)
> + return true;
> + }
> + return false;
> +}
> +
> +static void __evlist__disable(struct evlist *evlist, char *evsel_name)
> {
> struct evsel *pos;
> struct affinity affinity;
> @@ -386,6 +409,8 @@ void evlist__disable(struct evlist *evlist)
> affinity__set(&affinity, cpu);
>
> evlist__for_each_entry(evlist, pos) {
> + if (evsel__strcmp(pos, evsel_name))
> + continue;
> if (evsel__cpu_iter_skip(pos, cpu))
> continue;
> if (pos->disabled || !evsel__is_group_leader(pos) || !pos->core.fd)
> @@ -403,15 +428,34 @@ void evlist__disable(struct evlist *evlist)
>
> affinity__cleanup(&affinity);
> evlist__for_each_entry(evlist, pos) {
> + if (evsel__strcmp(pos, evsel_name))
> + continue;
> if (!evsel__is_group_leader(pos) || !pos->core.fd)
> continue;
> pos->disabled = true;
> }
>
> - evlist->enabled = false;
> + /*
> + * If we disabled only single event, we need to check
> + * the enabled state of the evlist manually.
> + */
> + if (evsel_name)
> + evlist->enabled = evlist__is_enabled(evlist);
> + else
> + evlist->enabled = false;
> +}
> +
> +void evlist__disable(struct evlist *evlist)
> +{
> + __evlist__disable(evlist, NULL);
> +}
> +
> +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name)
> +{
> + __evlist__disable(evlist, evsel_name);
> }
>
> -void evlist__enable(struct evlist *evlist)
> +static void __evlist__enable(struct evlist *evlist, char *evsel_name)
> {
> struct evsel *pos;
> struct affinity affinity;
> @@ -424,6 +468,8 @@ void evlist__enable(struct evlist *evlist)
> affinity__set(&affinity, cpu);
>
> evlist__for_each_entry(evlist, pos) {
> + if (evsel__strcmp(pos, evsel_name))
> + continue;
> if (evsel__cpu_iter_skip(pos, cpu))
> continue;
> if (!evsel__is_group_leader(pos) || !pos->core.fd)
> @@ -433,14 +479,31 @@ void evlist__enable(struct evlist *evlist)
> }
> affinity__cleanup(&affinity);
> evlist__for_each_entry(evlist, pos) {
> + if (evsel__strcmp(pos, evsel_name))
> + continue;
> if (!evsel__is_group_leader(pos) || !pos->core.fd)
> continue;
> pos->disabled = false;
> }
>
> + /*
> + * Even single event sets the 'enabled' for evlist,
> + * so the toggle can work properly and toggle to
> + * 'disabled' state.
> + */
> evlist->enabled = true;
> }
>
> +void evlist__enable(struct evlist *evlist)
> +{
> + __evlist__enable(evlist, NULL);
> +}
> +
> +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name)
> +{
> + __evlist__enable(evlist, evsel_name);
> +}
> +
> void evlist__toggle_enable(struct evlist *evlist)
> {
> (evlist->enabled ? evlist__disable : evlist__enable)(evlist);
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 9b0c795736bb..1aae75895dea 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -186,6 +186,8 @@ size_t evlist__mmap_size(unsigned long pages);
> void evlist__disable(struct evlist *evlist);
> void evlist__enable(struct evlist *evlist);
> void evlist__toggle_enable(struct evlist *evlist);
> +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name);
> +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name);
>
> int evlist__enable_event_idx(struct evlist *evlist, struct evsel *evsel, int idx);
>
> --
> 2.26.2
>
--
- Arnaldo