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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Jun 2022 09:35:32 -0400
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Ravi Bangoria <ravi.bangoria@....com>, acme@...nel.org
Cc:     jolsa@...nel.org, irogers@...gle.com, peterz@...radead.org,
        rrichter@....com, mingo@...hat.com, mark.rutland@....com,
        namhyung@...nel.org, tglx@...utronix.de, bp@...en8.de,
        james.clark@....com, leo.yan@...aro.org, ak@...ux.intel.com,
        eranian@...gle.com, like.xu.linux@...il.com, x86@...nel.org,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        sandipan.das@....com, ananth.narayan@....com, kim.phillips@....com,
        santosh.shukla@....com
Subject: Re: [PATCH v5 2/8] perf tool: Parse pmu caps sysfs only once



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@....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;