[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUnwY1BMw8epL+HS=grAmY6quRS5AZiED_HVDQZy2F8mg@mail.gmail.com>
Date: Fri, 8 Nov 2024 15:48:36 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
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>,
Ravi Bangoria <ravi.bangoria@....com>, Weilin Wang <weilin.wang@...el.com>,
Yoshihiro Furudera <fj5100bi@...itsu.com>, James Clark <james.clark@...aro.org>,
Athira Jajeev <atrajeev@...ux.vnet.ibm.com>, Howard Chu <howardchu95@...il.com>,
Oliver Upton <oliver.upton@...ux.dev>, Changbin Du <changbin.du@...wei.com>,
Ze Gao <zegao2021@...il.com>, Junhao He <hejunhao3@...wei.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v7 3/7] perf test: Add hwmon filename parser test
On Fri, Nov 8, 2024 at 11:34 AM Namhyung Kim <namhyung@...nel.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@...gle.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
> >
Powered by blists - more mailing lists