Re: [PATCH v5 2/8] perf tool: Parse pmu caps sysfs only once

["Liang, Kan" <kan.liang@linux.intel.com>]

On 5/31/2022 11:26 PM, Ravi Bangoria wrote:
> In addition to returning nr_caps, cache it locally in struct perf_pmu.
> Similarly, cache status of whether caps sysfs has already been parsed
> or not. These will help to avoid parsing sysfs every time the function
> gets called.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>   tools/perf/util/pmu.c | 15 +++++++++++----
>   tools/perf/util/pmu.h |  2 ++
>   2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 9a1c7e63e663..0112e1c36418 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1890,7 +1890,11 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>   	const char *sysfs = sysfs__mountpoint();
>   	DIR *caps_dir;
>   	struct dirent *evt_ent;
> -	int nr_caps = 0;
> +
> +	if (pmu->caps_initialized)
> +		return pmu->nr_caps;
> +
> +	pmu->nr_caps = 0;
>   
>   	if (!sysfs)
>   		return -1;
> @@ -1898,8 +1902,10 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>   	snprintf(caps_path, PATH_MAX,
>   		 "%s" EVENT_SOURCE_DEVICE_PATH "%s/caps", sysfs, pmu->name);
>   
> -	if (stat(caps_path, &st) < 0)
> +	if (stat(caps_path, &st) < 0) {
> +		pmu->caps_initialized = true;
>   		return 0;	/* no error if caps does not exist */
> +	}
>   
>   	caps_dir = opendir(caps_path);
>   	if (!caps_dir)
> @@ -1926,13 +1932,14 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>   			continue;
>   		}
>   
> -		nr_caps++;
> +		pmu->nr_caps++;
>   		fclose(file);
>   	}
>   
>   	closedir(caps_dir);
>   
> -	return nr_caps;
> +	pmu->caps_initialized = true;
> +	return pmu->nr_caps;
>   }
>   
>   void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 541889fa9f9c..4b45fd8da5a3 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -46,6 +46,8 @@ struct perf_pmu {
>   	struct perf_cpu_map *cpus;
>   	struct list_head format;  /* HEAD struct perf_pmu_format -> list */
>   	struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
> +	bool caps_initialized;
> +	u32 nr_caps;

If they are just used for the cache purpose, I don't think we need to 
add the variables in the struct perf_pmu.

A static variable should be good enough. See sysctl__nmi_watchdog_enabled().

Thanks,
Kan

>   	struct list_head caps;    /* HEAD struct perf_pmu_caps -> list */
>   	struct list_head list;    /* ELEM */
>   	struct list_head hybrid_list;