Re: [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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@kernel.org>
> Acked-by: Alexei Budankov <abudankov@huawei.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  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