1. Patchew
  2. linux
  3. [PATCH v7 0/7] Hwmon PMUs
  4. View patch

[PATCH v7 3/7] perf test: Add hwmon filename parser test

Ian Rogers posted 7 patches 2 weeks, 1 day ago
There is a newer version of this series
[PATCH v7 3/7] perf test: Add hwmon filename parser test
Posted by Ian Rogers 2 weeks, 1 day ago 
Filename parsing maps a hwmon filename to constituent parts enum/int
parts for the hwmon config value. Add a test case for the parsing.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   1 +
 tools/perf/tests/hwmon_pmu.c    | 110 ++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 tools/perf/util/hwmon_pmu.c     |   2 +-
 5 files changed, 114 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/hwmon_pmu.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 01ed9335db4d..ec4e1f034742 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -66,6 +66,7 @@ perf-test-y += sigtrap.o
 perf-test-y += event_groups.o
 perf-test-y += symbols.o
 perf-test-y += util.o
+perf-test-y += hwmon_pmu.o
 perf-test-y += tool_pmu.o
 
 ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index d2cabaa8ad92..8dcf74d3c0a3 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__PERF_RECORD,
 	&suite__pmu,
 	&suite__pmu_events,
+	&suite__hwmon_pmu,
 	&suite__tool_pmu,
 	&suite__dso_data,
 	&suite__perf_evsel__roundtrip_name_test,
diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
new file mode 100644
index 000000000000..f5b58486d8d3
--- /dev/null
+++ b/tools/perf/tests/hwmon_pmu.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "debug.h"
+#include "hwmon_pmu.h"
+#include "tests.h"
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+
+static int test__parse_hwmon_filename(struct test_suite *test __maybe_unused,
+				      int subtest __maybe_unused)
+{
+	const struct hwmon_parse_test {
+		const char *filename;
+		enum hwmon_type type;
+		int number;
+		enum hwmon_item item;
+		bool alarm;
+		bool parse_ok;
+	} tests[] = {
+		{
+			.filename = "cpu0_accuracy",
+			.type = HWMON_TYPE_CPU,
+			.number = 0,
+			.item = HWMON_ITEM_ACCURACY,
+			.alarm = false,
+			.parse_ok = true,
+		},
+		{
+			.filename = "temp1_input",
+			.type = HWMON_TYPE_TEMP,
+			.number = 1,
+			.item = HWMON_ITEM_INPUT,
+			.alarm = false,
+			.parse_ok = true,
+		},
+		{
+			.filename = "fan2_vid",
+			.type = HWMON_TYPE_FAN,
+			.number = 2,
+			.item = HWMON_ITEM_VID,
+			.alarm = false,
+			.parse_ok = true,
+		},
+		{
+			.filename = "power3_crit_alarm",
+			.type = HWMON_TYPE_POWER,
+			.number = 3,
+			.item = HWMON_ITEM_CRIT,
+			.alarm = true,
+			.parse_ok = true,
+		},
+		{
+			.filename = "intrusion4_average_interval_min_alarm",
+			.type = HWMON_TYPE_INTRUSION,
+			.number = 4,
+			.item = HWMON_ITEM_AVERAGE_INTERVAL_MIN,
+			.alarm = true,
+			.parse_ok = true,
+		},
+		{
+			.filename = "badtype5_baditem",
+			.type = HWMON_TYPE_NONE,
+			.number = 5,
+			.item = HWMON_ITEM_NONE,
+			.alarm = false,
+			.parse_ok = false,
+		},
+		{
+			.filename = "humidity6_baditem",
+			.type = HWMON_TYPE_NONE,
+			.number = 6,
+			.item = HWMON_ITEM_NONE,
+			.alarm = false,
+			.parse_ok = false,
+		},
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(tests); i++) {
+		enum hwmon_type type;
+		int number;
+		enum hwmon_item item;
+		bool alarm;
+
+		TEST_ASSERT_EQUAL("parse_hwmon_filename",
+				parse_hwmon_filename(
+					tests[i].filename,
+					&type,
+					&number,
+					&item,
+					&alarm),
+				tests[i].parse_ok
+			);
+		if (tests[i].parse_ok) {
+			TEST_ASSERT_EQUAL("parse_hwmon_filename type", type, tests[i].type);
+			TEST_ASSERT_EQUAL("parse_hwmon_filename number", number, tests[i].number);
+			TEST_ASSERT_EQUAL("parse_hwmon_filename item", item, tests[i].item);
+			TEST_ASSERT_EQUAL("parse_hwmon_filename alarm", alarm, tests[i].alarm);
+		}
+	}
+	return TEST_OK;
+}
+
+static struct test_case tests__hwmon_pmu[] = {
+	TEST_CASE("Basic parsing test", parse_hwmon_filename),
+	{	.name = NULL, }
+};
+
+struct test_suite suite__hwmon_pmu = {
+	.desc = "Hwmon PMU",
+	.test_cases = tests__hwmon_pmu,
+};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index af284dd47e5c..cb58b43aa063 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -92,6 +92,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
 DECLARE_SUITE(syscall_openat_tp_fields);
 DECLARE_SUITE(pmu);
 DECLARE_SUITE(pmu_events);
+DECLARE_SUITE(hwmon_pmu);
 DECLARE_SUITE(tool_pmu);
 DECLARE_SUITE(attr);
 DECLARE_SUITE(dso_data);
diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index ee5fb1c41da3..f4b7b3b6a052 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -126,7 +126,7 @@ bool parse_hwmon_filename(const char *filename,
 	fn_item_len = strlen(fn_item);
 	if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
 		assert(strlen(LONGEST_HWMON_ITEM_STR) < sizeof(fn_type));
-		strlcpy(fn_type, fn_item, fn_item_len - 6);
+		strlcpy(fn_type, fn_item, fn_item_len - 5);
 		fn_item = fn_type;
 		*alarm = true;
 	}
-- 
2.47.0.277.g8800431eea-goog
Re: [PATCH v7 3/7] perf test: Add hwmon filename parser test
Posted by Namhyung Kim 2 weeks, 1 day ago 
On Fri, Nov 08, 2024 at 09:49:32AM -0800, Ian Rogers wrote:
> Filename parsing maps a hwmon filename to constituent parts enum/int
> parts for the hwmon config value. Add a test case for the parsing.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/Build          |   1 +
>  tools/perf/tests/builtin-test.c |   1 +
>  tools/perf/tests/hwmon_pmu.c    | 110 ++++++++++++++++++++++++++++++++
>  tools/perf/tests/tests.h        |   1 +
>  tools/perf/util/hwmon_pmu.c     |   2 +-
>  5 files changed, 114 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/tests/hwmon_pmu.c
> 
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 01ed9335db4d..ec4e1f034742 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -66,6 +66,7 @@ perf-test-y += sigtrap.o
>  perf-test-y += event_groups.o
>  perf-test-y += symbols.o
>  perf-test-y += util.o
> +perf-test-y += hwmon_pmu.o
>  perf-test-y += tool_pmu.o
>  
>  ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index d2cabaa8ad92..8dcf74d3c0a3 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
>  	&suite__PERF_RECORD,
>  	&suite__pmu,
>  	&suite__pmu_events,
> +	&suite__hwmon_pmu,
>  	&suite__tool_pmu,
>  	&suite__dso_data,
>  	&suite__perf_evsel__roundtrip_name_test,
> diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
> new file mode 100644
> index 000000000000..f5b58486d8d3
> --- /dev/null
> +++ b/tools/perf/tests/hwmon_pmu.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +#include "debug.h"
> +#include "hwmon_pmu.h"
> +#include "tests.h"
> +#include <linux/compiler.h>
> +#include <linux/kernel.h>
> +
> +static int test__parse_hwmon_filename(struct test_suite *test __maybe_unused,
> +				      int subtest __maybe_unused)
> +{
> +	const struct hwmon_parse_test {
> +		const char *filename;
> +		enum hwmon_type type;
> +		int number;
> +		enum hwmon_item item;
> +		bool alarm;
> +		bool parse_ok;
> +	} tests[] = {
> +		{
> +			.filename = "cpu0_accuracy",
> +			.type = HWMON_TYPE_CPU,
> +			.number = 0,
> +			.item = HWMON_ITEM_ACCURACY,
> +			.alarm = false,
> +			.parse_ok = true,
> +		},
> +		{
> +			.filename = "temp1_input",
> +			.type = HWMON_TYPE_TEMP,
> +			.number = 1,
> +			.item = HWMON_ITEM_INPUT,
> +			.alarm = false,
> +			.parse_ok = true,
> +		},
> +		{
> +			.filename = "fan2_vid",
> +			.type = HWMON_TYPE_FAN,
> +			.number = 2,
> +			.item = HWMON_ITEM_VID,
> +			.alarm = false,
> +			.parse_ok = true,
> +		},
> +		{
> +			.filename = "power3_crit_alarm",
> +			.type = HWMON_TYPE_POWER,
> +			.number = 3,
> +			.item = HWMON_ITEM_CRIT,
> +			.alarm = true,
> +			.parse_ok = true,
> +		},
> +		{
> +			.filename = "intrusion4_average_interval_min_alarm",
> +			.type = HWMON_TYPE_INTRUSION,
> +			.number = 4,
> +			.item = HWMON_ITEM_AVERAGE_INTERVAL_MIN,
> +			.alarm = true,
> +			.parse_ok = true,
> +		},
> +		{
> +			.filename = "badtype5_baditem",
> +			.type = HWMON_TYPE_NONE,
> +			.number = 5,
> +			.item = HWMON_ITEM_NONE,
> +			.alarm = false,
> +			.parse_ok = false,
> +		},
> +		{
> +			.filename = "humidity6_baditem",
> +			.type = HWMON_TYPE_NONE,
> +			.number = 6,
> +			.item = HWMON_ITEM_NONE,
> +			.alarm = false,
> +			.parse_ok = false,
> +		},
> +	};
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(tests); i++) {
> +		enum hwmon_type type;
> +		int number;
> +		enum hwmon_item item;
> +		bool alarm;
> +
> +		TEST_ASSERT_EQUAL("parse_hwmon_filename",
> +				parse_hwmon_filename(
> +					tests[i].filename,
> +					&type,
> +					&number,
> +					&item,
> +					&alarm),
> +				tests[i].parse_ok
> +			);
> +		if (tests[i].parse_ok) {
> +			TEST_ASSERT_EQUAL("parse_hwmon_filename type", type, tests[i].type);
> +			TEST_ASSERT_EQUAL("parse_hwmon_filename number", number, tests[i].number);
> +			TEST_ASSERT_EQUAL("parse_hwmon_filename item", item, tests[i].item);
> +			TEST_ASSERT_EQUAL("parse_hwmon_filename alarm", alarm, tests[i].alarm);
> +		}
> +	}
> +	return TEST_OK;
> +}
> +
> +static struct test_case tests__hwmon_pmu[] = {
> +	TEST_CASE("Basic parsing test", parse_hwmon_filename),
> +	{	.name = NULL, }
> +};
> +
> +struct test_suite suite__hwmon_pmu = {
> +	.desc = "Hwmon PMU",
> +	.test_cases = tests__hwmon_pmu,
> +};
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index af284dd47e5c..cb58b43aa063 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -92,6 +92,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
>  DECLARE_SUITE(syscall_openat_tp_fields);
>  DECLARE_SUITE(pmu);
>  DECLARE_SUITE(pmu_events);
> +DECLARE_SUITE(hwmon_pmu);
>  DECLARE_SUITE(tool_pmu);
>  DECLARE_SUITE(attr);
>  DECLARE_SUITE(dso_data);
> diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> index ee5fb1c41da3..f4b7b3b6a052 100644
> --- a/tools/perf/util/hwmon_pmu.c
> +++ b/tools/perf/util/hwmon_pmu.c
> @@ -126,7 +126,7 @@ bool parse_hwmon_filename(const char *filename,
>  	fn_item_len = strlen(fn_item);
>  	if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
>  		assert(strlen(LONGEST_HWMON_ITEM_STR) < sizeof(fn_type));
> -		strlcpy(fn_type, fn_item, fn_item_len - 6);
> +		strlcpy(fn_type, fn_item, fn_item_len - 5);

Why is it changed?

Thanks,
Namhyung


>  		fn_item = fn_type;
>  		*alarm = true;
>  	}
> -- 
> 2.47.0.277.g8800431eea-goog
>
Re: [PATCH v7 3/7] perf test: Add hwmon filename parser test
Posted by Ian Rogers 2 weeks ago 
On Fri, Nov 8, 2024 at 11:34 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Nov 08, 2024 at 09:49:32AM -0800, Ian Rogers wrote:
> > Filename parsing maps a hwmon filename to constituent parts enum/int
> > parts for the hwmon config value. Add a test case for the parsing.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/Build          |   1 +
> >  tools/perf/tests/builtin-test.c |   1 +
> >  tools/perf/tests/hwmon_pmu.c    | 110 ++++++++++++++++++++++++++++++++
> >  tools/perf/tests/tests.h        |   1 +
> >  tools/perf/util/hwmon_pmu.c     |   2 +-
> >  5 files changed, 114 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/perf/tests/hwmon_pmu.c
> >
> > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > index 01ed9335db4d..ec4e1f034742 100644
> > --- a/tools/perf/tests/Build
> > +++ b/tools/perf/tests/Build
> > @@ -66,6 +66,7 @@ perf-test-y += sigtrap.o
> >  perf-test-y += event_groups.o
> >  perf-test-y += symbols.o
> >  perf-test-y += util.o
> > +perf-test-y += hwmon_pmu.o
> >  perf-test-y += tool_pmu.o
> >
> >  ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index d2cabaa8ad92..8dcf74d3c0a3 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -71,6 +71,7 @@ static struct test_suite *generic_tests[] = {
> >       &suite__PERF_RECORD,
> >       &suite__pmu,
> >       &suite__pmu_events,
> > +     &suite__hwmon_pmu,
> >       &suite__tool_pmu,
> >       &suite__dso_data,
> >       &suite__perf_evsel__roundtrip_name_test,
> > diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
> > new file mode 100644
> > index 000000000000..f5b58486d8d3
> > --- /dev/null
> > +++ b/tools/perf/tests/hwmon_pmu.c
> > @@ -0,0 +1,110 @@
> > +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> > +#include "debug.h"
> > +#include "hwmon_pmu.h"
> > +#include "tests.h"
> > +#include <linux/compiler.h>
> > +#include <linux/kernel.h>
> > +
> > +static int test__parse_hwmon_filename(struct test_suite *test __maybe_unused,
> > +                                   int subtest __maybe_unused)
> > +{
> > +     const struct hwmon_parse_test {
> > +             const char *filename;
> > +             enum hwmon_type type;
> > +             int number;
> > +             enum hwmon_item item;
> > +             bool alarm;
> > +             bool parse_ok;
> > +     } tests[] = {
> > +             {
> > +                     .filename = "cpu0_accuracy",
> > +                     .type = HWMON_TYPE_CPU,
> > +                     .number = 0,
> > +                     .item = HWMON_ITEM_ACCURACY,
> > +                     .alarm = false,
> > +                     .parse_ok = true,
> > +             },
> > +             {
> > +                     .filename = "temp1_input",
> > +                     .type = HWMON_TYPE_TEMP,
> > +                     .number = 1,
> > +                     .item = HWMON_ITEM_INPUT,
> > +                     .alarm = false,
> > +                     .parse_ok = true,
> > +             },
> > +             {
> > +                     .filename = "fan2_vid",
> > +                     .type = HWMON_TYPE_FAN,
> > +                     .number = 2,
> > +                     .item = HWMON_ITEM_VID,
> > +                     .alarm = false,
> > +                     .parse_ok = true,
> > +             },
> > +             {
> > +                     .filename = "power3_crit_alarm",
> > +                     .type = HWMON_TYPE_POWER,
> > +                     .number = 3,
> > +                     .item = HWMON_ITEM_CRIT,
> > +                     .alarm = true,
> > +                     .parse_ok = true,
> > +             },
> > +             {
> > +                     .filename = "intrusion4_average_interval_min_alarm",
> > +                     .type = HWMON_TYPE_INTRUSION,
> > +                     .number = 4,
> > +                     .item = HWMON_ITEM_AVERAGE_INTERVAL_MIN,
> > +                     .alarm = true,
> > +                     .parse_ok = true,
> > +             },
> > +             {
> > +                     .filename = "badtype5_baditem",
> > +                     .type = HWMON_TYPE_NONE,
> > +                     .number = 5,
> > +                     .item = HWMON_ITEM_NONE,
> > +                     .alarm = false,
> > +                     .parse_ok = false,
> > +             },
> > +             {
> > +                     .filename = "humidity6_baditem",
> > +                     .type = HWMON_TYPE_NONE,
> > +                     .number = 6,
> > +                     .item = HWMON_ITEM_NONE,
> > +                     .alarm = false,
> > +                     .parse_ok = false,
> > +             },
> > +     };
> > +
> > +     for (size_t i = 0; i < ARRAY_SIZE(tests); i++) {
> > +             enum hwmon_type type;
> > +             int number;
> > +             enum hwmon_item item;
> > +             bool alarm;
> > +
> > +             TEST_ASSERT_EQUAL("parse_hwmon_filename",
> > +                             parse_hwmon_filename(
> > +                                     tests[i].filename,
> > +                                     &type,
> > +                                     &number,
> > +                                     &item,
> > +                                     &alarm),
> > +                             tests[i].parse_ok
> > +                     );
> > +             if (tests[i].parse_ok) {
> > +                     TEST_ASSERT_EQUAL("parse_hwmon_filename type", type, tests[i].type);
> > +                     TEST_ASSERT_EQUAL("parse_hwmon_filename number", number, tests[i].number);
> > +                     TEST_ASSERT_EQUAL("parse_hwmon_filename item", item, tests[i].item);
> > +                     TEST_ASSERT_EQUAL("parse_hwmon_filename alarm", alarm, tests[i].alarm);
> > +             }
> > +     }
> > +     return TEST_OK;
> > +}
> > +
> > +static struct test_case tests__hwmon_pmu[] = {
> > +     TEST_CASE("Basic parsing test", parse_hwmon_filename),
> > +     {       .name = NULL, }
> > +};
> > +
> > +struct test_suite suite__hwmon_pmu = {
> > +     .desc = "Hwmon PMU",
> > +     .test_cases = tests__hwmon_pmu,
> > +};
> > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> > index af284dd47e5c..cb58b43aa063 100644
> > --- a/tools/perf/tests/tests.h
> > +++ b/tools/perf/tests/tests.h
> > @@ -92,6 +92,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
> >  DECLARE_SUITE(syscall_openat_tp_fields);
> >  DECLARE_SUITE(pmu);
> >  DECLARE_SUITE(pmu_events);
> > +DECLARE_SUITE(hwmon_pmu);
> >  DECLARE_SUITE(tool_pmu);
> >  DECLARE_SUITE(attr);
> >  DECLARE_SUITE(dso_data);
> > diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> > index ee5fb1c41da3..f4b7b3b6a052 100644
> > --- a/tools/perf/util/hwmon_pmu.c
> > +++ b/tools/perf/util/hwmon_pmu.c
> > @@ -126,7 +126,7 @@ bool parse_hwmon_filename(const char *filename,
> >       fn_item_len = strlen(fn_item);
> >       if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
> >               assert(strlen(LONGEST_HWMON_ITEM_STR) < sizeof(fn_type));
> > -             strlcpy(fn_type, fn_item, fn_item_len - 6);
> > +             strlcpy(fn_type, fn_item, fn_item_len - 5);
>
> Why is it changed?

Should be part of the last commit, there is an off-by-1 caused by
strlcpy counting copying the \0. I'll resend.

Thanks,
Ian

> Thanks,
> Namhyung
>
>
> >               fn_item = fn_type;
> >               *alarm = true;
> >       }
> > --
> > 2.47.0.277.g8800431eea-goog
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.