linux-kernel - Re: [PATCH v1] perf tests: Fix hwmon parsing with PMU name test

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]

Message-ID: <Zz7KAAG2pHhCg-Dx@google.com>
Date: Wed, 20 Nov 2024 21:49:52 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] perf tests: Fix hwmon parsing with PMU name test

On Wed, Nov 20, 2024 at 04:09:55PM -0800, Ian Rogers wrote:
> Incorrectly the hwmon with PMU name test didn't pass "true". Fix and
> address issue with hwmon_pmu__config_terms needing to load events - a
> load bearing assert fired. Also fix missing list deletion when putting
> the hwmon test PMU and lower some debug warnings to make the hwmon PMU
> less spammy in verbose mode.
> 
> Signed-off-by: Ian Rogers <irogers@...gle.com>

Tested-by: Namhyung Kim <namhyung@...nel.org>

Thanks,
Namhyung

> ---
>  tools/perf/tests/hwmon_pmu.c |  5 +++--
>  tools/perf/util/hwmon_pmu.c  | 16 ++++++++++------
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
> index d8bc71b51599..b4656529146e 100644
> --- a/tools/perf/tests/hwmon_pmu.c
> +++ b/tools/perf/tests/hwmon_pmu.c
> @@ -41,6 +41,7 @@ static int test_pmu_put(const char *dir, struct perf_pmu *hwm)
>  	if (ret)
>  		pr_err("Failure to \"%s\"\n", buf);
>  
> +	list_del(&hwm->list);
>  	perf_pmu__delete(hwm);
>  	return ret;
>  }
> @@ -147,7 +148,7 @@ static int do_test(size_t i, bool with_pmu, bool with_alias)
>  	}
>  
>  	if (with_pmu)
> -		snprintf(str, sizeof(str), "/%s/", test_event);
> +		snprintf(str, sizeof(str), "hwmon_a_test_hwmon_pmu/%s/", test_event);
>  	else
>  		strlcpy(str, test_event, sizeof(str));
>  
> @@ -230,7 +231,7 @@ static int test__hwmon_pmu_without_pmu(struct test_suite *test __maybe_unused,
>  static int test__hwmon_pmu_with_pmu(struct test_suite *test __maybe_unused,
>  				   int subtest __maybe_unused)
>  {
> -	return test__hwmon_pmu(/*with_pmu=*/false);
> +	return test__hwmon_pmu(/*with_pmu=*/true);
>  }
>  
>  static int test__parse_hwmon_filename(struct test_suite *test __maybe_unused,
> diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> index 4d9d6f405434..e61429b38ba7 100644
> --- a/tools/perf/util/hwmon_pmu.c
> +++ b/tools/perf/util/hwmon_pmu.c
> @@ -197,13 +197,13 @@ bool parse_hwmon_filename(const char *filename,
>  		}
>  	}
>  	if (fn_item == NULL || fn_type[0] == '\0' || (item != NULL && fn_item[0] == '\0')) {
> -		pr_debug("hwmon_pmu: not a hwmon file '%s'\n", filename);
> +		pr_debug3("hwmon_pmu: not a hwmon file '%s'\n", filename);
>  		return false;
>  	}
>  	elem = bsearch(&fn_type, hwmon_type_strs + 1, ARRAY_SIZE(hwmon_type_strs) - 1,
>  		       sizeof(hwmon_type_strs[0]), hwmon_strcmp);
>  	if (!elem) {
> -		pr_debug("hwmon_pmu: not a hwmon type '%s' in file name '%s'\n",
> +		pr_debug3("hwmon_pmu: not a hwmon type '%s' in file name '%s'\n",
>  			 fn_type, filename);
>  		return false;
>  	}
> @@ -223,7 +223,7 @@ bool parse_hwmon_filename(const char *filename,
>  	elem = bsearch(fn_item, hwmon_item_strs + 1, ARRAY_SIZE(hwmon_item_strs) - 1,
>  		       sizeof(hwmon_item_strs[0]), hwmon_strcmp);
>  	if (!elem) {
> -		pr_debug("hwmon_pmu: not a hwmon item '%s' in file name '%s'\n",
> +		pr_debug3("hwmon_pmu: not a hwmon item '%s' in file name '%s'\n",
>  			 fn_item, filename);
>  		return false;
>  	}
> @@ -281,7 +281,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
>  			continue;
>  
>  		if (!parse_hwmon_filename(ent->d_name, &type, &number, &item, &alarm)) {
> -			pr_debug("Not a hwmon file '%s'\n", ent->d_name);
> +			pr_debug3("Not a hwmon file '%s'\n", ent->d_name);
>  			continue;
>  		}
>  		key.num = number;
> @@ -653,10 +653,14 @@ int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
>  			    struct parse_events_terms *terms,
>  			    struct parse_events_error *err)
>  {
> -	const struct hwmon_pmu *hwm = container_of(pmu, struct hwmon_pmu, pmu);
> +	struct hwmon_pmu *hwm = container_of(pmu, struct hwmon_pmu, pmu);
>  	struct parse_events_term *term;
> +	int ret;
> +
> +	ret = hwmon_pmu__read_events(hwm);
> +	if (ret)
> +		return ret;
>  
> -	assert(pmu->sysfs_aliases_loaded);
>  	list_for_each_entry(term, &terms->terms, list) {
>  		if (hwmon_pmu__config_term(hwm, attr, term, err))
>  			return -EINVAL;
> -- 
> 2.47.0.371.ga323438b13-goog
>