* [PATCH v4 10/11] Documentation: Add kunit_shutdown to kernel-parameters.txt
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add kunit_shutdown, an option to specify that the kernel shutsdown after
running KUnit tests, to the kernel-parameters.txt documentation.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79a..e7d5eb7249e7f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2183,6 +2183,14 @@
0: force disabled
1: force enabled
+ kunit_shutdown=[KERNEL UNIT TESTING FRAMEWORK] Shutdown kernel after
+ running built-in tests. Tests configured as modules will
+ not be run.
+ Default: (flag not present) don't shutdown
+ poweroff: poweroff the kernel after running tests
+ halt: halt the kernel after running tests
+ reboot: reboot the kernel after running tests
+
kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs.
Default is 0 (don't ignore, but inject #GP)
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 09/11] kunit: test: add test plan to KUnit TAP format
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
TAP 14 allows an optional test plan to be emitted before the start of
the start of testing[1]; this is valuable because it makes it possible
for a test harness to detect whether the number of tests run matches the
number of tests expected to be run, ensuring that no tests silently
failed.
Link[1]: https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md#the-plan
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
lib/kunit/executor.c | 37 +++++++++
lib/kunit/test.c | 11 ---
tools/testing/kunit/kunit_kernel.py | 2 +-
tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++---
.../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes
.../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes
.../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes
7 files changed, 100 insertions(+), 24 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 4aab7f70a88c3..38061d456afb2 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/reboot.h>
#include <kunit/test.h>
/*
@@ -11,15 +12,51 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
#if IS_BUILTIN(CONFIG_KUNIT)
+static char *kunit_shutdown;
+core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
+
+static void kunit_handle_shutdown(void)
+{
+ if (!kunit_shutdown)
+ return;
+
+ if (!strcmp(kunit_shutdown, "poweroff"))
+ kernel_power_off();
+ else if (!strcmp(kunit_shutdown, "halt"))
+ kernel_halt();
+ else if (!strcmp(kunit_shutdown, "reboot"))
+ kernel_restart(NULL);
+
+}
+
+static void kunit_print_tap_header(void)
+{
+ struct kunit_suite * const * const *suites, * const *subsuite;
+ int num_of_suites = 0;
+
+ for (suites = __kunit_suites_start;
+ suites < __kunit_suites_end;
+ suites++)
+ for (subsuite = *suites; *subsuite != NULL; subsuite++)
+ num_of_suites++;
+
+ pr_info("TAP version 14\n");
+ pr_info("1..%d\n", num_of_suites);
+}
+
int kunit_run_all_tests(void)
{
struct kunit_suite * const * const *suites;
+ kunit_print_tap_header();
+
for (suites = __kunit_suites_start;
suites < __kunit_suites_end;
suites++)
__kunit_test_suites_init(*suites);
+ kunit_handle_shutdown();
+
return 0;
}
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 918dff400a9d7..b1835ccb3fce2 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -19,16 +19,6 @@ static void kunit_set_failure(struct kunit *test)
WRITE_ONCE(test->success, false);
}
-static void kunit_print_tap_version(void)
-{
- static bool kunit_has_printed_tap_version;
-
- if (!kunit_has_printed_tap_version) {
- pr_info("TAP version 14\n");
- kunit_has_printed_tap_version = true;
- }
-}
-
/*
* Append formatted message to log, size of which is limited to
* KUNIT_LOG_SIZE bytes (including null terminating byte).
@@ -68,7 +58,6 @@ EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
static void kunit_print_subtest_start(struct kunit_suite *suite)
{
- kunit_print_tap_version();
kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "# Subtest: %s",
suite->name);
kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "1..%zd",
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 63dbda2d029f6..d6a575f92317c 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -172,7 +172,7 @@ class LinuxSourceTree(object):
return self.validate_config(build_dir)
def run_kernel(self, args=[], build_dir='', timeout=None):
- args.extend(['mem=1G'])
+ args.extend(['mem=1G', 'kunit_shutdown=halt'])
outfile = 'test.log'
self._ops.linux_bin(args, timeout, build_dir, outfile)
subprocess.call(['stty', 'sane'])
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 64aac9dcd4314..a8998a5effaad 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -45,6 +45,7 @@ class TestStatus(Enum):
FAILURE = auto()
TEST_CRASHED = auto()
NO_TESTS = auto()
+ FAILURE_TO_PARSE_TESTS = auto()
kunit_start_re = re.compile(r'TAP version [0-9]+$')
kunit_end_re = re.compile('(List of all partitions:|'
@@ -109,7 +110,7 @@ OkNotOkResult = namedtuple('OkNotOkResult', ['is_ok','description', 'text'])
OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$')
-OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) [0-9]+ - (.*)$')
+OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$')
def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool:
save_non_diagnositic(lines, test_case)
@@ -197,7 +198,9 @@ def max_status(left: TestStatus, right: TestStatus) -> TestStatus:
else:
return TestStatus.SUCCESS
-def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool:
+def parse_ok_not_ok_test_suite(lines: List[str],
+ test_suite: TestSuite,
+ expected_suite_index: int) -> bool:
consume_non_diagnositic(lines)
if not lines:
test_suite.status = TestStatus.TEST_CRASHED
@@ -210,6 +213,12 @@ def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool:
test_suite.status = TestStatus.SUCCESS
else:
test_suite.status = TestStatus.FAILURE
+ suite_index = int(match.group(2))
+ if suite_index != expected_suite_index:
+ print_with_timestamp(
+ red('[ERROR] ') + 'expected_suite_index ' +
+ str(expected_suite_index) + ', but got ' +
+ str(suite_index))
return True
else:
return False
@@ -222,7 +231,7 @@ def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus:
max_test_case_status = bubble_up_errors(lambda x: x.status, test_suite.cases)
return max_status(max_test_case_status, test_suite.status)
-def parse_test_suite(lines: List[str]) -> TestSuite:
+def parse_test_suite(lines: List[str], expected_suite_index: int) -> TestSuite:
if not lines:
return None
consume_non_diagnositic(lines)
@@ -241,7 +250,7 @@ def parse_test_suite(lines: List[str]) -> TestSuite:
break
test_suite.cases.append(test_case)
expected_test_case_num -= 1
- if parse_ok_not_ok_test_suite(lines, test_suite):
+ if parse_ok_not_ok_test_suite(lines, test_suite, expected_suite_index):
test_suite.status = bubble_up_test_case_errors(test_suite)
return test_suite
elif not lines:
@@ -261,6 +270,17 @@ def parse_tap_header(lines: List[str]) -> bool:
else:
return False
+TEST_PLAN = re.compile(r'[0-9]+\.\.([0-9]+)')
+
+def parse_test_plan(lines: List[str]) -> int:
+ consume_non_diagnositic(lines)
+ match = TEST_PLAN.match(lines[0])
+ if match:
+ lines.pop(0)
+ return int(match.group(1))
+ else:
+ return None
+
def bubble_up_suite_errors(test_suite_list: List[TestSuite]) -> TestStatus:
return bubble_up_errors(lambda x: x.status, test_suite_list)
@@ -269,19 +289,34 @@ def parse_test_result(lines: List[str]) -> TestResult:
return TestResult(TestStatus.NO_TESTS, [], lines)
consume_non_diagnositic(lines)
if not parse_tap_header(lines):
- return None
+ return TestResult(TestStatus.NO_TESTS, [], lines)
+ expected_test_suite_num = parse_test_plan(lines)
+ if not expected_test_suite_num:
+ return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines)
test_suites = []
- test_suite = parse_test_suite(lines)
- while test_suite:
- test_suites.append(test_suite)
- test_suite = parse_test_suite(lines)
- return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines)
+ for i in range(1, expected_test_suite_num + 1):
+ test_suite = parse_test_suite(lines, i)
+ if test_suite:
+ test_suites.append(test_suite)
+ else:
+ print_with_timestamp(
+ red('[ERROR] ') + ' expected ' +
+ str(expected_test_suite_num) +
+ ' test suites, but got ' + str(i - 2))
+ break
+ test_suite = parse_test_suite(lines, -1)
+ if test_suite:
+ print_with_timestamp(red('[ERROR] ') +
+ 'got unexpected test suite: ' + test_suite.name)
+ if test_suites:
+ return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines)
+ else:
+ return TestResult(TestStatus.NO_TESTS, [], lines)
-def parse_run_tests(kernel_output) -> TestResult:
+def print_and_count_results(test_result: TestResult) -> None:
total_tests = 0
failed_tests = 0
crashed_tests = 0
- test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
for test_suite in test_result.suites:
if test_suite.status == TestStatus.SUCCESS:
print_suite_divider(green('[PASSED] ') + test_suite.name)
@@ -303,6 +338,21 @@ def parse_run_tests(kernel_output) -> TestResult:
print_with_timestamp(red('[FAILED] ') + test_case.name)
print_log(map(yellow, test_case.log))
print_with_timestamp('')
+ return total_tests, failed_tests, crashed_tests
+
+def parse_run_tests(kernel_output) -> TestResult:
+ total_tests = 0
+ failed_tests = 0
+ crashed_tests = 0
+ test_result = parse_test_result(list(isolate_kunit_output(kernel_output)))
+ if test_result.status == TestStatus.NO_TESTS:
+ print(red('[ERROR] ') + yellow('no tests run!'))
+ elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS:
+ print(red('[ERROR] ') + yellow('could not parse test results!'))
+ else:
+ (total_tests,
+ failed_tests,
+ crashed_tests) = print_and_count_results(test_result)
print_with_timestamp(DIVIDER)
fmt = green if test_result.status == TestStatus.SUCCESS else red
print_with_timestamp(
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log b/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log
index 62ebc0288355c4b122ccc18ae2505f971efa57bc..bc0dc8fe35b760b1feb74ec419818dbfae1adb5c 100644
GIT binary patch
delta 28
jcmbQmGoME|#4$jjEVZaOGe1wk(1goSPtRy09}gP<dC~`u
delta 23
ecmbQwGmD2W#4$jjEVZaOGe1wk&}5@94;uhhkp{*9
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-crash.log b/tools/testing/kunit/test_data/test_is_test_passed-crash.log
index 0b249870c8be417a5865bd40a24c8597bb7f5ab1..4d97f6708c4a5ad5bb2ac879e12afca6e816d83d 100644
GIT binary patch
delta 15
WcmX>hepY;fFN>j`p3z318g2k9Uj*m?
delta 10
RcmX>renNbL@5Z2NZU7lr1S$Xk
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-failure.log b/tools/testing/kunit/test_data/test_is_test_passed-failure.log
index 9e89d32d5667a59d137f8adacf3a88fdb7f88baf..7a416497e3bec044eefc1535f7d84ee85703ba97 100644
GIT binary patch
delta 28
jcmZ3&yOLKp#4$jjEVZaOGe1wk(1goSPtRy0-!wJ=eKrU$
delta 23
ecmZ3<yM&i7#4$jjEVZaOGe1wk&}5_VG&TTPhX-Z=
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 08/11] init: main: add KUnit to kernel init
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Remove KUnit from init calls entirely, instead call directly from
kernel_init().
Co-developed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
include/kunit/test.h | 9 +++++++++
init/main.c | 4 ++++
lib/kunit/executor.c | 4 +---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index d13965eb624d4..7cb1c47388c56 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -228,6 +228,15 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites);
void __kunit_test_suites_exit(struct kunit_suite **suites);
+#if IS_BUILTIN(CONFIG_KUNIT)
+int kunit_run_all_tests(void);
+#else
+static inline int kunit_run_all_tests(void)
+{
+ return 0;
+}
+#endif /* IS_BUILTIN(CONFIG_KUNIT) */
+
/**
* kunit_test_suites() - used to register one or more &struct kunit_suite
* with KUnit.
diff --git a/init/main.c b/init/main.c
index 0ead83e86b5aa..d3101d8874dea 100644
--- a/init/main.c
+++ b/init/main.c
@@ -106,6 +106,8 @@
#define CREATE_TRACE_POINTS
#include <trace/events/initcall.h>
+#include <kunit/test.h>
+
static int kernel_init(void *);
extern void init_IRQ(void);
@@ -1504,6 +1506,8 @@ static noinline void __init kernel_init_freeable(void)
do_basic_setup();
+ kunit_run_all_tests();
+
console_on_rootfs();
/*
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 7015e7328dce7..4aab7f70a88c3 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -11,7 +11,7 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
#if IS_BUILTIN(CONFIG_KUNIT)
-static int kunit_run_all_tests(void)
+int kunit_run_all_tests(void)
{
struct kunit_suite * const * const *suites;
@@ -23,6 +23,4 @@ static int kunit_run_all_tests(void)
return 0;
}
-late_initcall(kunit_run_all_tests);
-
#endif /* IS_BUILTIN(CONFIG_KUNIT) */
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 07/11] kunit: test: create a single centralized executor for all tests
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
From: Alan Maguire <alan.maguire@oracle.com>
Add a centralized executor to dispatch tests rather than relying on
late_initcall to schedule each test suite separately. Centralized
execution is for built-in tests only; modules will execute tests
when loaded.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Co-developed-by: Iurii Zaikin <yzaikin@google.com>
Signed-off-by: Iurii Zaikin <yzaikin@google.com>
Co-developed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
include/kunit/test.h | 64 ++++++++++++++++++++++++++++++--------------
lib/kunit/Makefile | 3 ++-
lib/kunit/executor.c | 28 +++++++++++++++++++
lib/kunit/test.c | 2 +-
4 files changed, 75 insertions(+), 22 deletions(-)
create mode 100644 lib/kunit/executor.c
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 47e61e1d53370..d13965eb624d4 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -224,7 +224,7 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
unsigned int kunit_test_case_num(struct kunit_suite *suite,
struct kunit_case *test_case);
-int __kunit_test_suites_init(struct kunit_suite **suites);
+int __kunit_test_suites_init(struct kunit_suite * const * const suites);
void __kunit_test_suites_exit(struct kunit_suite **suites);
@@ -237,34 +237,58 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);
* Registers @suites_list with the test framework. See &struct kunit_suite for
* more information.
*
- * When builtin, KUnit tests are all run as late_initcalls; this means
- * that they cannot test anything where tests must run at a different init
- * phase. One significant restriction resulting from this is that KUnit
- * cannot reliably test anything that is initialize in the late_init phase;
- * another is that KUnit is useless to test things that need to be run in
- * an earlier init phase.
- *
- * An alternative is to build the tests as a module. Because modules
- * do not support multiple late_initcall()s, we need to initialize an
- * array of suites for a module.
- *
- * TODO(brendanhiggins@google.com): Don't run all KUnit tests as
- * late_initcalls. I have some future work planned to dispatch all KUnit
- * tests from the same place, and at the very least to do so after
- * everything else is definitely initialized.
+ * If a test suite is built-in, module_init() gets translated into
+ * an initcall which we don't want as the idea is that for builtins
+ * the executor will manage execution. So ensure we do not define
+ * module_{init|exit} functions for the builtin case when registering
+ * suites via kunit_test_suites() below.
*/
-#define kunit_test_suites(suites_list...) \
- static struct kunit_suite *suites[] = {suites_list, NULL}; \
- static int kunit_test_suites_init(void) \
+#ifdef MODULE
+#define kunit_test_suites_for_module(__suites) \
+ static int __init kunit_test_suites_init(void) \
{ \
+ struct kunit_suite *suites[] = (__suites); \
return __kunit_test_suites_init(suites); \
} \
- late_initcall(kunit_test_suites_init); \
+ module_init(kunit_test_suites_init); \
+ \
static void __exit kunit_test_suites_exit(void) \
{ \
return __kunit_test_suites_exit(suites); \
} \
module_exit(kunit_test_suites_exit)
+#else
+#define kunit_test_suites_for_module(__suites)
+#endif /* MODULE */
+
+#define __kunit_test_suites(unique_array, unique_suites, ...) \
+ static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \
+ kunit_test_suites_for_module(unique_array); \
+ static struct kunit_suite **unique_suites \
+ __used __section(.kunit_test_suites) = unique_array
+
+/**
+ * kunit_test_suites() - used to register one or more &struct kunit_suite
+ * with KUnit.
+ *
+ * @suites: a statically allocated list of &struct kunit_suite.
+ *
+ * Registers @suites with the test framework. See &struct kunit_suite for
+ * more information.
+ *
+ * When builtin, KUnit tests are all run via executor; this is done
+ * by placing the array of struct kunit_suite * in the .kunit_test_suites
+ * ELF section.
+ *
+ * An alternative is to build the tests as a module. Because modules do not
+ * support multiple initcall()s, we need to initialize an array of suites for a
+ * module.
+ *
+ */
+#define kunit_test_suites(...) \
+ __kunit_test_suites(__UNIQUE_ID(array), \
+ __UNIQUE_ID(suites), \
+ __VA_ARGS__)
#define kunit_test_suite(suite) kunit_test_suites(&suite)
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 724b94311ca36..c49f4ffb6273a 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o
kunit-objs += test.o \
string-stream.o \
assert.o \
- try-catch.o
+ try-catch.o \
+ executor.o
ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
kunit-objs += debugfs.o
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
new file mode 100644
index 0000000000000..7015e7328dce7
--- /dev/null
+++ b/lib/kunit/executor.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <kunit/test.h>
+
+/*
+ * These symbols point to the .kunit_test_suites section and are defined in
+ * include/asm-generic/vmlinux.lds.h, and consequently must be extern.
+ */
+extern struct kunit_suite * const * const __kunit_suites_start[];
+extern struct kunit_suite * const * const __kunit_suites_end[];
+
+#if IS_BUILTIN(CONFIG_KUNIT)
+
+static int kunit_run_all_tests(void)
+{
+ struct kunit_suite * const * const *suites;
+
+ for (suites = __kunit_suites_start;
+ suites < __kunit_suites_end;
+ suites++)
+ __kunit_test_suites_init(*suites);
+
+ return 0;
+}
+
+late_initcall(kunit_run_all_tests);
+
+#endif /* IS_BUILTIN(CONFIG_KUNIT) */
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index ccb2ffad8dcfa..918dff400a9d7 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -380,7 +380,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
kunit_debugfs_create_suite(suite);
}
-int __kunit_test_suites_init(struct kunit_suite **suites)
+int __kunit_test_suites_init(struct kunit_suite * const * const suites)
{
unsigned int i;
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 06/11] arch: xtensa: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section to xtensa where KUnit can put references to its
test suites. This patch is an early step in transitioning to dispatching
all KUnit tests from a centralized executor rather than having each as
its own separate late_initcall.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
arch/xtensa/kernel/vmlinux.lds.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/xtensa/kernel/vmlinux.lds.S b/arch/xtensa/kernel/vmlinux.lds.S
index d23a6e38f0625..9aec4ef67d0b0 100644
--- a/arch/xtensa/kernel/vmlinux.lds.S
+++ b/arch/xtensa/kernel/vmlinux.lds.S
@@ -216,6 +216,10 @@ SECTIONS
INIT_RAM_FS
}
+ .kunit_test_suites : {
+ KUNIT_TEST_SUITES
+ }
+
PERCPU_SECTION(XCHAL_ICACHE_LINESIZE)
/* We need this dummy segment here */
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 05/11] arch: um: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section to UML where KUnit can put references to its test
suites. This patch is an early step in transitioning to dispatching all
KUnit tests from a centralized executor rather than having each as its
own separate late_initcall.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
arch/um/include/asm/common.lds.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S
index eca6c452a41bd..9a9c97f45694c 100644
--- a/arch/um/include/asm/common.lds.S
+++ b/arch/um/include/asm/common.lds.S
@@ -52,6 +52,10 @@
CON_INITCALL
}
+ .kunit_test_suites : {
+ KUNIT_TEST_SUITES
+ }
+
.exitcall : {
__exitcall_begin = .;
*(.exitcall.exit)
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 03/11] arch: microblaze: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section to microblaze where KUnit can put references to its
test suites. This patch is an early step in transitioning to dispatching
all KUnit tests from a centralized executor rather than having each as
its own separate late_initcall.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
arch/microblaze/kernel/vmlinux.lds.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S
index df07b3d06cd6b..4fc32f8979a60 100644
--- a/arch/microblaze/kernel/vmlinux.lds.S
+++ b/arch/microblaze/kernel/vmlinux.lds.S
@@ -128,6 +128,10 @@ SECTIONS {
__init_end = .;
+ .kunit_test_suites : {
+ KUNIT_TEST_SUITES
+ }
+
.bss ALIGN (PAGE_SIZE) : AT(ADDR(.bss) - LOAD_OFFSET) {
/* page aligned when MMU used */
__bss_start = . ;
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* Re: [PATCH V3 0/4] mm/debug_vm_pgtable: Add some more tests
From: Alexander Gordeev @ 2020-06-24 14:40 UTC (permalink / raw)
To: Gerald Schaefer
Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, ziy, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Anshuman Khandual, Borislav Petkov, Paul Walmsley,
Kirill A . Shutemov, Thomas Gleixner, linux-arm-kernel,
christophe.leroy, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200624134808.0c460862@thinkpad>
On Wed, Jun 24, 2020 at 01:48:08PM +0200, Gerald Schaefer wrote:
> On Wed, 24 Jun 2020 13:05:39 +0200
> Alexander Gordeev <agordeev@linux.ibm.com> wrote:
>
> > On Wed, Jun 24, 2020 at 08:43:10AM +0530, Anshuman Khandual wrote:
> >
> > [...]
> >
> > > Hello Gerald/Christophe/Vineet,
> > >
> > > It would be really great if you could give this series a quick test
> > > on s390/ppc/arc platforms respectively. Thank you.
> >
> > That worked for me with the default and debug s390 configurations.
> > Would you like to try with some particular options or combinations
> > of the options?
>
> It will be enabled automatically on all archs that set
> ARCH_HAS_DEBUG_VM_PGTABLE, which we do for s390 unconditionally.
> Also, DEBUG_VM has to be set, which we have only in the debug config.
> So only the s390 debug config will have it enabled, you can check
> dmesg for "debug_vm_pgtable" to see when / where it was run, and if it
> triggered any warnings.
Yes, that is what I did ;)
I should have been more clear. I wonder whether Anshuman has in
mind other options which possibly makes sense to set or unset
and check how it goes with non-standard configurations.
> I also checked with the v3 series, and it works fine for s390.
^ permalink raw reply
* Re: [PATCH V3 0/4] mm/debug_vm_pgtable: Add some more tests
From: Alexander Gordeev @ 2020-06-24 11:05 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, linux-arm-kernel, ziy, Catalin Marinas,
linux-snps-arc, Vasily Gorbik, Borislav Petkov, Paul Walmsley,
Kirill A . Shutemov, Thomas Gleixner, gerald.schaefer,
christophe.leroy, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <70ddc7dd-b688-b73e-642a-6363178c8cdd@arm.com>
On Wed, Jun 24, 2020 at 08:43:10AM +0530, Anshuman Khandual wrote:
[...]
> Hello Gerald/Christophe/Vineet,
>
> It would be really great if you could give this series a quick test
> on s390/ppc/arc platforms respectively. Thank you.
That worked for me with the default and debug s390 configurations.
Would you like to try with some particular options or combinations
of the options?
> - Anshuman
^ permalink raw reply
* Re: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
From: Alexander Gordeev @ 2020-06-24 8:23 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, linuxppc-dev, R.T.Dickinson, mad skateman,
Christian Zigotzky
In-Reply-To: <56DB95B8-5F42-4837-ABA0-7E580FE04B73@xenosoft.de>
On Sun, Jun 21, 2020 at 08:40:14AM +0200, Christian Zigotzky wrote:
> Hello Alexander,
>
> The DPAA Ethernet doesn’t work anymore on our FSL P5020/P5040 boards [1] since the RC1 of kernel 5.8 [2].
> We bisected last days [3] and found the problematic commit [4]. I was able to revert it [5]. After that the DPAA Ethernet works again. I created a patch for reverting the commit [5]. This patch works and I will use it for the RC2.
> Could you please check your commit? [4]
Hi Christian,
Could you please check if the kernel passes CONFIG_TEST_BITMAP self-test?
Thanks!
> Thanks,
> Christian
>
> [1] http://wiki.amiga.org/index.php?title=X5000
> [2] https://forum.hyperion-entertainment.com/viewtopic.php?p=50885#p50885
> [3] https://forum.hyperion-entertainment.com/viewtopic.php?p=50892#p50892
> [4] lib: fix bitmap_parse() on 64-bit big endian archs: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=81c4f4d924d5d009b5ed785a3e22b18d0f7b831f
> [5] https://forum.hyperion-entertainment.com/viewtopic.php?p=50982#p50982
>
>
^ permalink raw reply
* [PATCH v4 02/11] arch: arm64: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section to arm64 where KUnit can put references to its test
suites. This patch is an early step in transitioning to dispatching all
KUnit tests from a centralized executor rather than having each as its
own separate late_initcall.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
arch/arm64/kernel/vmlinux.lds.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 6827da7f3aa54..a1cae9cc655d7 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -181,6 +181,9 @@ SECTIONS
INIT_RAM_FS
*(.init.rodata.* .init.bss) /* from the EFI stub */
}
+ .kunit_test_suites : {
+ KUNIT_TEST_SUITES
+ }
.exit.data : {
EXIT_DATA
}
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 04/11] arch: powerpc: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section to powerpc where KUnit can put references to its test
suites. This patch is an early step in transitioning to dispatching all
KUnit tests from a centralized executor rather than having each as its
own separate late_initcall.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
arch/powerpc/kernel/vmlinux.lds.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 326e113d2e456..0cc97dbfde0ad 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -202,6 +202,10 @@ SECTIONS
CON_INITCALL
}
+ .kunit_test_suites : {
+ KUNIT_TEST_SUITES
+ }
+
. = ALIGN(8);
__ftr_fixup : AT(ADDR(__ftr_fixup) - LOAD_OFFSET) {
__start___ftr_fixup = .;
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* [PATCH v4 00/11] kunit: create a centralized executor to dispatch all KUnit tests
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
## TL;DR
This patchset adds a centralized executor to dispatch tests rather than
relying on late_initcall to schedule each test suite separately along
with a couple of new features that depend on it.
Also, sorry for the extreme delay in getting this out. Part of the delay
came from finding that there were actually several architectures that
the previous revision of this patchset didn't work on, so I went through
and attempted to test this patchset on every architecture - more on that
later.
## What am I trying to do?
Conceptually, I am trying to provide a mechanism by which test suites
can be grouped together so that they can be reasoned about collectively.
The last two of three patches in this series add features which depend
on this:
PATCH 8/11 Prints out a test plan[1] right before KUnit tests are run;
this is valuable because it makes it possible for a test
harness to detect whether the number of tests run matches the
number of tests expected to be run, ensuring that no tests
silently failed. The test plan includes a count of tests that
will run. With the centralized executor, the tests are
located in a single data structure and thus can be counted.
PATCH 9/11 Add a new kernel command-line option which allows the user to
specify that the kernel poweroff, halt, or reboot after
completing all KUnit tests; this is very handy for running
KUnit tests on UML or a VM so that the UML/VM process exits
cleanly immediately after running all tests without needing a
special initramfs. The centralized executor provides a
definitive point when all tests have completed and the
poweroff, halt, or reboot could occur.
In addition, by dispatching tests from a single location, we can
guarantee that all KUnit tests run after late_init is complete, which
was a concern during the initial KUnit patchset review (this has not
been a problem in practice, but resolving with certainty is nevertheless
desirable).
Other use cases for this exist, but the above features should provide an
idea of the value that this could provide.
## Changes since last revision:
- On the last revision I got some messages from 0day that showed that
this patchset didn't work on several architectures, one issue that
this patchset addresses is that we were aligning both memory segments
as well as structures in the segments to specific byte boundaries
which was incorrect.
- The issue mentioned above also caused me to test on additional
architectures which revealed that some architectures other than UML
do not use the default init linker section macro that most
architectures use. There are now several new patches (2, 3, 4, and
6).
- Fixed a formatting consistency issue in the kernel params
documentation patch (9/9).
- Add a brief blurb on how and when the kunit_test_suite macro works.
## Remaining work to be done:
The only architecture for which I was able to get a compiler, but was
apparently unable to get KUnit into a section that the executor to see
was m68k - not sure why.
Alan Maguire (1):
kunit: test: create a single centralized executor for all tests
Brendan Higgins (10):
vmlinux.lds.h: add linker section for KUnit test suites
arch: arm64: add linker section for KUnit test suites
arch: microblaze: add linker section for KUnit test suites
arch: powerpc: add linker section for KUnit test suites
arch: um: add linker section for KUnit test suites
arch: xtensa: add linker section for KUnit test suites
init: main: add KUnit to kernel init
kunit: test: add test plan to KUnit TAP format
Documentation: Add kunit_shutdown to kernel-parameters.txt
Documentation: kunit: add a brief blurb about kunit_test_suite
.../admin-guide/kernel-parameters.txt | 8 ++
Documentation/dev-tools/kunit/usage.rst | 5 ++
arch/arm64/kernel/vmlinux.lds.S | 3 +
arch/microblaze/kernel/vmlinux.lds.S | 4 +
arch/powerpc/kernel/vmlinux.lds.S | 4 +
arch/um/include/asm/common.lds.S | 4 +
arch/xtensa/kernel/vmlinux.lds.S | 4 +
include/asm-generic/vmlinux.lds.h | 8 ++
include/kunit/test.h | 73 ++++++++++++-----
init/main.c | 4 +
lib/kunit/Makefile | 3 +-
lib/kunit/executor.c | 63 +++++++++++++++
lib/kunit/test.c | 13 +--
tools/testing/kunit/kunit_kernel.py | 2 +-
tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++---
.../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes
.../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes
.../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes
18 files changed, 226 insertions(+), 46 deletions(-)
create mode 100644 lib/kunit/executor.c
base-commit: 4333a9b0b67bb4e8bcd91bdd80da80b0ec151162
prerequisite-patch-id: 2d4b5aa9fa8ada9ae04c8584b47c299a822b9455
prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0
These patches are available for download with dependencies here:
https://kunit-review.googlesource.com/c/linux/+/3829
[1] https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md#the-plan
[2] https://patchwork.kernel.org/patch/11383635/
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply
* [PATCH v4 01/11] vmlinux.lds.h: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-06-24 20:55 UTC (permalink / raw)
To: jdike, richard, anton.ivanov, arnd, keescook, skhan, alan.maguire,
yzaikin, davidgow, akpm, rppt, frowand.list, catalin.marinas,
will, monstr, mpe, benh, paulus, chris, jcmvbkbc
Cc: linux-arch, linux-xtensa, linux-doc, sboyd, gregkh, linuxppc-dev,
linux-um, linux-kernel, Brendan Higgins, mcgrof, linux-kselftest,
logang, linux-arm-kernel, kunit-dev
In-Reply-To: <20200624205550.215599-1-brendanhiggins@google.com>
Add a linker section where KUnit can put references to its test suites.
This patch is the first step in transitioning to dispatching all KUnit
tests from a centralized executor rather than having each as its own
separate late_initcall.
Co-developed-by: Iurii Zaikin <yzaikin@google.com>
Signed-off-by: Iurii Zaikin <yzaikin@google.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
include/asm-generic/vmlinux.lds.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index db600ef218d7d..4f9b036fc9616 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -881,6 +881,13 @@
KEEP(*(.con_initcall.init)) \
__con_initcall_end = .;
+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
+#define KUNIT_TEST_SUITES \
+ . = ALIGN(8); \
+ __kunit_suites_start = .; \
+ KEEP(*(.kunit_test_suites)) \
+ __kunit_suites_end = .;
+
#ifdef CONFIG_BLK_DEV_INITRD
#define INIT_RAM_FS \
. = ALIGN(4); \
@@ -1056,6 +1063,7 @@
INIT_CALLS \
CON_INITCALL \
INIT_RAM_FS \
+ KUNIT_TEST_SUITES \
}
#define BSS_SECTION(sbss_align, bss_align, stop_align) \
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related
* Re: [PATCH v4 6/8] arm: Break cyclic percpu include
From: Peter Zijlstra @ 2020-06-24 17:53 UTC (permalink / raw)
To: Will Deacon
Cc: linux-s390, bigeasy, x86, heiko.carstens, linux-kernel, rostedt,
davem, a.darwish, sparclinux, linux, tglx, linuxppc-dev, mingo
In-Reply-To: <20200623090257.GA3743@willie-the-truck>
On Tue, Jun 23, 2020 at 10:02:57AM +0100, Will Deacon wrote:
> On Tue, Jun 23, 2020 at 10:36:51AM +0200, Peter Zijlstra wrote:
> > In order to use <asm/percpu.h> in irqflags.h, we need to make sure
> > asm/percpu.h does not itself depend on irqflags.h.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > arch/arm/include/asm/percpu.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > --- a/arch/arm/include/asm/percpu.h
> > +++ b/arch/arm/include/asm/percpu.h
> > @@ -10,6 +10,8 @@
> > * in the TPIDRPRW. TPIDRPRW only exists on V6K and V7
> > */
> > #if defined(CONFIG_SMP) && !defined(CONFIG_CPU_V6)
> > +register unsigned long current_stack_pointer asm ("sp");
>
> If you define this unconditionally, then we can probably get rid of the
> copy in asm/thread_info.h, rather than duplicate the same #define.
The below delta seems to build arm-allnoconfig, arm-defconfig and
arm-allmodconfig.
Although please don't ask me how asm/thread_info.h includes asm/percpu.h
Does that work for you?
diff --git a/arch/arm/include/asm/percpu.h b/arch/arm/include/asm/percpu.h
index e86e47486b6b1..e2fcb3cfd3de5 100644
--- a/arch/arm/include/asm/percpu.h
+++ b/arch/arm/include/asm/percpu.h
@@ -5,13 +5,13 @@
#ifndef _ASM_ARM_PERCPU_H_
#define _ASM_ARM_PERCPU_H_
+register unsigned long current_stack_pointer asm ("sp");
+
/*
* Same as asm-generic/percpu.h, except that we store the per cpu offset
* in the TPIDRPRW. TPIDRPRW only exists on V6K and V7
*/
#if defined(CONFIG_SMP) && !defined(CONFIG_CPU_V6)
-register unsigned long current_stack_pointer asm ("sp");
-
static inline void set_my_cpu_offset(unsigned long off)
{
/* Set TPIDRPRW */
diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 3609a6980c342..536b6b979f634 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -75,11 +75,6 @@ struct thread_info {
.addr_limit = KERNEL_DS, \
}
-/*
- * how to get the current stack pointer in C
- */
-register unsigned long current_stack_pointer asm ("sp");
-
/*
* how to get the thread information struct from C
*/
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Ira Weiny @ 2020-06-24 17:33 UTC (permalink / raw)
To: Vaibhav Jain; +Cc: Aneesh Kumar K . V, linuxppc-dev, linux-nvdimm
In-Reply-To: <877dvwo6ha.fsf@linux.ibm.com>
On Wed, Jun 24, 2020 at 08:28:57PM +0530, Vaibhav Jain wrote:
> Thanks for reviewing this patch Ira,
>
> My responses below inline.
>
> Ira Weiny <ira.weiny@intel.com> writes:
>
> > On Mon, Jun 22, 2020 at 09:54:50AM +0530, Vaibhav Jain wrote:
> >> Update papr_scm.c to query dimm performance statistics from PHYP via
> >> H_SCM_PERFORMANCE_STATS hcall and export them to user-space as PAPR
> >> specific NVDIMM attribute 'perf_stats' in sysfs. The patch also
> >> provide a sysfs ABI documentation for the stats being reported and
> >> their meanings.
> >>
> >> During NVDIMM probe time in papr_scm_nvdimm_init() a special variant
> >> of H_SCM_PERFORMANCE_STATS hcall is issued to check if collection of
> >> performance statistics is supported or not. If successful then a PHYP
> >> returns a maximum possible buffer length needed to read all
> >> performance stats. This returned value is stored in a per-nvdimm
> >> attribute 'len_stat_buffer'.
> >>
> >> The layout of request buffer for reading NVDIMM performance stats from
> >> PHYP is defined in 'struct papr_scm_perf_stats' and 'struct
> >> papr_scm_perf_stat'. These structs are used in newly introduced
> >> drc_pmem_query_stats() that issues the H_SCM_PERFORMANCE_STATS hcall.
> >>
> >> The sysfs access function perf_stats_show() uses value
> >> 'len_stat_buffer' to allocate a buffer large enough to hold all
> >> possible NVDIMM performance stats and passes it to
> >> drc_pmem_query_stats() to populate. Finally statistics reported in the
> >> buffer are formatted into the sysfs access function output buffer.
> >>
> >> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> >> ---
> >> Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 ++++
> >> arch/powerpc/platforms/pseries/papr_scm.c | 139 ++++++++++++++++++
> >> 2 files changed, 166 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
> >> index 5b10d036a8d4..c1a67275c43f 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
> >> +++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
> >> @@ -25,3 +25,30 @@ Description:
> >> NVDIMM have been scrubbed.
> >> * "locked" : Indicating that NVDIMM contents cant
> >> be modified until next power cycle.
> >> +
> >> +What: /sys/bus/nd/devices/nmemX/papr/perf_stats
> >> +Date: May, 2020
> >> +KernelVersion: v5.9
> >> +Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
> >> +Description:
> >> + (RO) Report various performance stats related to papr-scm NVDIMM
> >> + device. Each stat is reported on a new line with each line
> >> + composed of a stat-identifier followed by it value. Below are
> >> + currently known dimm performance stats which are reported:
> >> +
> >> + * "CtlResCt" : Controller Reset Count
> >> + * "CtlResTm" : Controller Reset Elapsed Time
> >> + * "PonSecs " : Power-on Seconds
> >> + * "MemLife " : Life Remaining
> >> + * "CritRscU" : Critical Resource Utilization
> >> + * "HostLCnt" : Host Load Count
> >> + * "HostSCnt" : Host Store Count
> >> + * "HostSDur" : Host Store Duration
> >> + * "HostLDur" : Host Load Duration
> >> + * "MedRCnt " : Media Read Count
> >> + * "MedWCnt " : Media Write Count
> >> + * "MedRDur " : Media Read Duration
> >> + * "MedWDur " : Media Write Duration
> >> + * "CchRHCnt" : Cache Read Hit Count
> >> + * "CchWHCnt" : Cache Write Hit Count
> >> + * "FastWCnt" : Fast Write Count
> >> \ No newline at end of file
> >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> >> index 9c569078a09f..cb3f9acc325b 100644
> >> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> >> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> >> @@ -62,6 +62,24 @@
> >> PAPR_PMEM_HEALTH_FATAL | \
> >> PAPR_PMEM_HEALTH_UNHEALTHY)
> >>
> >> +#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
> >> +#define PAPR_SCM_PERF_STATS_VERSION 0x1
> >> +
> >> +/* Struct holding a single performance metric */
> >> +struct papr_scm_perf_stat {
> >> + u8 statistic_id[8];
> >> + u64 statistic_value;
> >> +};
> >> +
> >> +/* Struct exchanged between kernel and PHYP for fetching drc perf stats */
> >> +struct papr_scm_perf_stats {
> >> + u8 eye_catcher[8];
> >> + u32 stats_version; /* Should be 0x01 */
> > ^^^^
> > PAPR_SCM_PERF_STATS_VERSION?
> Sure. Will update in v2
>
> >
> >> + u32 num_statistics; /* Number of stats following */
> >> + /* zero or more performance matrics */
> >> + struct papr_scm_perf_stat scm_statistic[];
> >> +} __packed;
> >> +
> >> /* private struct associated with each region */
> >> struct papr_scm_priv {
> >> struct platform_device *pdev;
> >> @@ -89,6 +107,9 @@ struct papr_scm_priv {
> >>
> >> /* Health information for the dimm */
> >> u64 health_bitmap;
> >> +
> >> + /* length of the stat buffer as expected by phyp */
> >> + size_t len_stat_buffer;
> >> };
> >>
> >> static int drc_pmem_bind(struct papr_scm_priv *p)
> >> @@ -194,6 +215,75 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
> >> return drc_pmem_bind(p);
> >> }
> >>
> >> +/*
> >> + * Query the Dimm performance stats from PHYP and copy them (if returned) to
> >> + * provided struct papr_scm_perf_stats instance 'stats' of 'size' in bytes.
> >> + * The value of R4 is copied to 'out' if the pointer is provided.
> >> + */
> >> +static int drc_pmem_query_stats(struct papr_scm_priv *p,
> >> + struct papr_scm_perf_stats *buff_stats,
> >> + size_t size, unsigned int num_stats,
> >> + uint64_t *out)
> >> +{
> >> + unsigned long ret[PLPAR_HCALL_BUFSIZE];
> >> + struct papr_scm_perf_stat *stats;
> >> + s64 rc, i;
> >> +
> >> + /* Setup the out buffer */
> >> + if (buff_stats) {
> >> + memcpy(buff_stats->eye_catcher,
> >> + PAPR_SCM_PERF_STATS_EYECATCHER, 8);
> >> + buff_stats->stats_version =
> >> + cpu_to_be32(PAPR_SCM_PERF_STATS_VERSION);
> >> + buff_stats->num_statistics =
> >> + cpu_to_be32(num_stats);
> >> + } else {
> >> + /* In case of no out buffer ignore the size */
> >> + size = 0;
> >> + }
> >> +
> >> + /*
> >> + * Do the HCALL asking PHYP for info and if R4 was requested
> >> + * return its value in 'out' variable.
> >> + */
> >> + rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
> >> + virt_to_phys(buff_stats), size);
> >
> > You are calling virt_to_phys(NULL) here when called from
> > papr_scm_nvdimm_init()! That can't be right.
> Thanks for cathing this. However if the 'size' is '0' the 'buff_stats'
> address is ignored by the hypervisor hence this didnt get caught in my
> tests. Though CONFIG_DEBUG_VIRTUAL would have caught it early.
>
> >
> >> + if (out)
> >> + *out = ret[0];
> >> +
> >> + if (rc == H_PARTIAL) {
> >> + dev_err(&p->pdev->dev,
> >> + "Unknown performance stats, Err:0x%016lX\n", ret[0]);
> >> + return -ENOENT;
> >> + } else if (rc != H_SUCCESS) {
> >> + dev_err(&p->pdev->dev,
> >> + "Failed to query performance stats, Err:%lld\n", rc);
> >> + return -ENXIO;
> >> + }
> >> +
> >> + /* Successfully fetched the requested stats from phyp */
> >> + if (size != 0) {
> >> + buff_stats->num_statistics =
> >> + be32_to_cpu(buff_stats->num_statistics);
> >> +
> >> + /* Transform the stats buffer values from BE to cpu native */
> >> + for (i = 0, stats = buff_stats->scm_statistic;
> >> + i < buff_stats->num_statistics; ++i) {
> >> + stats[i].statistic_value =
> >> + be64_to_cpu(stats[i].statistic_value);
> >> + }
> >> + dev_dbg(&p->pdev->dev,
> >> + "Performance stats returned %d stats\n",
> >> + buff_stats->num_statistics);
> >> + } else {
> >> + /* Handle case where stat buffer size was requested */
> >> + dev_dbg(&p->pdev->dev,
> >> + "Performance stats size %ld\n", ret[0]);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> /*
> >> * Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
> >> * health information.
> >> @@ -631,6 +721,45 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
> >> return 0;
> >> }
> >>
> >> +static ssize_t perf_stats_show(struct device *dev,
> >> + struct device_attribute *attr, char *buf)
> >> +{
> >> + int index, rc;
> >> + struct seq_buf s;
> >> + struct papr_scm_perf_stat *stat;
> >> + struct papr_scm_perf_stats *stats;
> >> + struct nvdimm *dimm = to_nvdimm(dev);
> >> + struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> >> +
> >> + if (!p->len_stat_buffer)
> >> + return -ENOENT;
> >> +
> >> + /* Allocate the buffer for phyp where stats are written */
> >> + stats = kzalloc(p->len_stat_buffer, GFP_KERNEL);
> >
> > I'm concerned that this buffer does not seem to have anything to do with the
> > 'num_stats' parameter passed to drc_pmem_query_stats(). Furthermore why is
> > num_stats always 0 in those calls?
> >
> 'num_stats == 0' is a special case of the hcall where PHYP returns all
> the possible stats in the 'stats' buffer.
So how does the above allocate ensure that the buffer length is big enough to
cover all possible stats with this special case?
Ok I think I see that len_stat_buffer is set below after a query (presumably to
the hardware).
>
> >> + if (!stats)
> >> + return -ENOMEM;
> >> +
> >> + /* Ask phyp to return all dimm perf stats */
> >> + rc = drc_pmem_query_stats(p, stats, p->len_stat_buffer, 0, NULL);
> >> + if (!rc) {
> >> + /*
> >> + * Go through the returned output buffer and print stats and
> >> + * values. Since statistic_id is essentially a char string of
> >> + * 8 bytes, simply use the string format specifier to print it.
> >> + */
> >> + seq_buf_init(&s, buf, PAGE_SIZE);
> >> + for (index = 0, stat = stats->scm_statistic;
> >> + index < stats->num_statistics; ++index, ++stat) {
> >> + seq_buf_printf(&s, "%.8s = 0x%016llX\n",
> >> + stat->statistic_id, stat->statistic_value);
> >> + }
> >> + }
> >> +
> >> + kfree(stats);
> >> + return rc ? rc : seq_buf_used(&s);
> >> +}
> >> +DEVICE_ATTR_RO(perf_stats);
> >> +
> >> static ssize_t flags_show(struct device *dev,
> >> struct device_attribute *attr, char *buf)
> >> {
> >> @@ -676,6 +805,7 @@ DEVICE_ATTR_RO(flags);
> >> /* papr_scm specific dimm attributes */
> >> static struct attribute *papr_nd_attributes[] = {
> >> &dev_attr_flags.attr,
> >> + &dev_attr_perf_stats.attr,
> >> NULL,
> >> };
> >>
> >> @@ -696,6 +826,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> >> struct nd_region_desc ndr_desc;
> >> unsigned long dimm_flags;
> >> int target_nid, online_nid;
> >> + u64 stat_size;
> >>
> >> p->bus_desc.ndctl = papr_scm_ndctl;
> >> p->bus_desc.module = THIS_MODULE;
> >> @@ -759,6 +890,14 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> >> dev_info(dev, "Region registered with target node %d and online node %d",
> >> target_nid, online_nid);
> >>
> >> + /* Try retriving the stat buffer and see if its supported */
> >> + if (!drc_pmem_query_stats(p, NULL, 0, 0, &stat_size)) {
> >> + p->len_stat_buffer = (size_t)stat_size;
> >> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> >> + p->len_stat_buffer);
> >> + } else {
> >> + dev_info(&p->pdev->dev, "Limited dimm stat info available\n");
> >
> > Do we really need this print?
> nvdimm performance stats can be selectively turned on/off from the
> hypervisor management console hence this info message is more like a
> warning indicating that extended dimm stat info like 'fuel_gauge' is not
> available.
Ah... But this is saying that the stat info _is_ available? ("info available")
Should this be dev_warn(..., "... info not available\n")?
Ira
>
> >
> > Ira
> >
> >> + }
> >> return 0;
> >>
> >> err: nvdimm_bus_unregister(p->bus);
> >> --
> >> 2.26.2
> >> _______________________________________________
> >> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> >> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
>
> --
> Cheers
> ~ Vaibhav
^ permalink raw reply
* Re: [PATCH v2 0/6] Remove default DMA window before creating DDW
From: Leonardo Bras @ 2020-06-24 15:54 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Alexey Kardashevskiy, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200624062411.367796-1-leobras.c@gmail.com>
On Wed, 2020-06-24 at 03:24 -0300, Leonardo Bras wrote:
> Patch #5:
> Instead of destroying the created DDW if it doesn't map the whole
> partition, make use of it instead of the default DMA window.
>
> Patch #6:
> Changes the way iommu_bypass_supported_pSeriesLP() check for
> iommu_bypass: instead of checking the address returned by enable_ddw(),
> it checks a new output value that reflects if the DDW created maps
> the whole partition.
Patches #5 and #6 were sent more as a RFC, but since they depend on the
series, I decided adding them here.
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: fsl-asoc-card: Add WM8524 support
From: Mark Brown @ 2020-06-24 15:38 UTC (permalink / raw)
To: nicoleotsuka, festevam, linuxppc-dev, alsa-devel, Xiubo.Lee,
lgirdwood, timur, tiwai, perex, Shengjiu Wang, linux-kernel
In-Reply-To: <1592895167-30483-1-git-send-email-shengjiu.wang@nxp.com>
On Tue, 23 Jun 2020 14:52:46 +0800, Shengjiu Wang wrote:
> WM8524 only supports playback mode, and only works at
> slave mode.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: fsl-asoc-card: Add WM8524 support
commit: 3cd990267401fc7d0b222fc81f637e75e46967e0
[2/2] ASoC: bindings: fsl-asoc-card: Add compatible string for wm8524
commit: 3b3372fa65bab619f99bcfe272e92fb6faa07219
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH v2 2/2] cpufreq: Specify default governor on command line
From: Quentin Perret @ 2020-06-24 15:32 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Juri Lelli, Cc: Android Kernel, Vincent Guittot, Arnd Bergmann,
Linux PM, Peter Zijlstra, Viresh Kumar, adharmap,
Rafael J. Wysocki, Linux Kernel Mailing List, Ingo Molnar,
Paul Mackerras, linuxppc-dev, Todd Kjos
In-Reply-To: <CAJZ5v0ja_rM7i=psW1HRyzEpW=8QwP2u9p+ihN3FS8_53bbxTQ@mail.gmail.com>
On Wednesday 24 Jun 2020 at 14:51:04 (+0200), Rafael J. Wysocki wrote:
> On Wed, Jun 24, 2020 at 7:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > @@ -2789,7 +2796,13 @@ static int __init cpufreq_core_init(void)
> > > cpufreq_global_kobject = kobject_create_and_add("cpufreq", &cpu_subsys.dev_root->kobj);
> > > BUG_ON(!cpufreq_global_kobject);
> > >
> > > + mutex_lock(&cpufreq_governor_mutex);
> > > + if (!default_governor)
> > > + default_governor = cpufreq_default_governor();
> > > + mutex_unlock(&cpufreq_governor_mutex);
> >
> > I don't think locking is required here at core-initcall level.
>
> It isn't necessary AFAICS, but it may as well be regarded as
> annotation (kind of instead of having a comment explaining why it need
> not be used).
Right, but I must admit that, looking at this more, I'm getting a bit
confused with the overall locking for governors :/
When in cpufreq_init_policy() we find a governor using
find_governor(policy->last_governor), what guarantees this governor is
not concurrently unregistered? That is, what guarantees this governor
doesn't go away between that find_governor() call, and the subsequent
call to try_module_get() in cpufreq_set_policy() down the line?
Can we somewhat assume that whatever governor is referred to by
policy->last_governor will have a non-null refcount? Or are the
cpufreq_online() and cpufreq_unregister_governor() path mutually
exclusive? Or is there something else?
Thanks,
Quentin
^ permalink raw reply
* Re: [PATCH v4 7/8] lockdep: Change hardirq{s_enabled,_context} to per-cpu variables
From: Peter Zijlstra @ 2020-06-24 15:18 UTC (permalink / raw)
To: Marco Elver
Cc: linux-s390, linuxppc-dev, paulmck, bigeasy, x86, heiko.carstens,
linux-kernel, rostedt, davem, Ahmed S. Darwish, sparclinux, linux,
tglx, will, mingo
In-Reply-To: <20200624113246.GA170324@elver.google.com>
On Wed, Jun 24, 2020 at 01:32:46PM +0200, Marco Elver wrote:
> From: Marco Elver <elver@google.com>
> Date: Wed, 24 Jun 2020 11:23:22 +0200
> Subject: [PATCH] kcsan: Make KCSAN compatible with new IRQ state tracking
>
> The new IRQ state tracking code does not honor lockdep_off(), and as
> such we should again permit tracing by using non-raw functions in
> core.c. Update the lockdep_off() comment in report.c, to reflect the
> fact there is still a potential risk of deadlock due to using printk()
> from scheduler code.
>
> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Marco Elver <elver@google.com>
Thanks!
I've put this in front of the series at hand. I'll wait a little while
longer for arch people to give feedback on their header patches before I
stuff the lot into tip/locking/core.
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Vaibhav Jain @ 2020-06-24 14:58 UTC (permalink / raw)
To: Ira Weiny; +Cc: Aneesh Kumar K . V, linuxppc-dev, linux-nvdimm
In-Reply-To: <20200623190255.GG3910394@iweiny-DESK2.sc.intel.com>
Thanks for reviewing this patch Ira,
My responses below inline.
Ira Weiny <ira.weiny@intel.com> writes:
> On Mon, Jun 22, 2020 at 09:54:50AM +0530, Vaibhav Jain wrote:
>> Update papr_scm.c to query dimm performance statistics from PHYP via
>> H_SCM_PERFORMANCE_STATS hcall and export them to user-space as PAPR
>> specific NVDIMM attribute 'perf_stats' in sysfs. The patch also
>> provide a sysfs ABI documentation for the stats being reported and
>> their meanings.
>>
>> During NVDIMM probe time in papr_scm_nvdimm_init() a special variant
>> of H_SCM_PERFORMANCE_STATS hcall is issued to check if collection of
>> performance statistics is supported or not. If successful then a PHYP
>> returns a maximum possible buffer length needed to read all
>> performance stats. This returned value is stored in a per-nvdimm
>> attribute 'len_stat_buffer'.
>>
>> The layout of request buffer for reading NVDIMM performance stats from
>> PHYP is defined in 'struct papr_scm_perf_stats' and 'struct
>> papr_scm_perf_stat'. These structs are used in newly introduced
>> drc_pmem_query_stats() that issues the H_SCM_PERFORMANCE_STATS hcall.
>>
>> The sysfs access function perf_stats_show() uses value
>> 'len_stat_buffer' to allocate a buffer large enough to hold all
>> possible NVDIMM performance stats and passes it to
>> drc_pmem_query_stats() to populate. Finally statistics reported in the
>> buffer are formatted into the sysfs access function output buffer.
>>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 ++++
>> arch/powerpc/platforms/pseries/papr_scm.c | 139 ++++++++++++++++++
>> 2 files changed, 166 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
>> index 5b10d036a8d4..c1a67275c43f 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
>> +++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
>> @@ -25,3 +25,30 @@ Description:
>> NVDIMM have been scrubbed.
>> * "locked" : Indicating that NVDIMM contents cant
>> be modified until next power cycle.
>> +
>> +What: /sys/bus/nd/devices/nmemX/papr/perf_stats
>> +Date: May, 2020
>> +KernelVersion: v5.9
>> +Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
>> +Description:
>> + (RO) Report various performance stats related to papr-scm NVDIMM
>> + device. Each stat is reported on a new line with each line
>> + composed of a stat-identifier followed by it value. Below are
>> + currently known dimm performance stats which are reported:
>> +
>> + * "CtlResCt" : Controller Reset Count
>> + * "CtlResTm" : Controller Reset Elapsed Time
>> + * "PonSecs " : Power-on Seconds
>> + * "MemLife " : Life Remaining
>> + * "CritRscU" : Critical Resource Utilization
>> + * "HostLCnt" : Host Load Count
>> + * "HostSCnt" : Host Store Count
>> + * "HostSDur" : Host Store Duration
>> + * "HostLDur" : Host Load Duration
>> + * "MedRCnt " : Media Read Count
>> + * "MedWCnt " : Media Write Count
>> + * "MedRDur " : Media Read Duration
>> + * "MedWDur " : Media Write Duration
>> + * "CchRHCnt" : Cache Read Hit Count
>> + * "CchWHCnt" : Cache Write Hit Count
>> + * "FastWCnt" : Fast Write Count
>> \ No newline at end of file
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 9c569078a09f..cb3f9acc325b 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -62,6 +62,24 @@
>> PAPR_PMEM_HEALTH_FATAL | \
>> PAPR_PMEM_HEALTH_UNHEALTHY)
>>
>> +#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
>> +#define PAPR_SCM_PERF_STATS_VERSION 0x1
>> +
>> +/* Struct holding a single performance metric */
>> +struct papr_scm_perf_stat {
>> + u8 statistic_id[8];
>> + u64 statistic_value;
>> +};
>> +
>> +/* Struct exchanged between kernel and PHYP for fetching drc perf stats */
>> +struct papr_scm_perf_stats {
>> + u8 eye_catcher[8];
>> + u32 stats_version; /* Should be 0x01 */
> ^^^^
> PAPR_SCM_PERF_STATS_VERSION?
Sure. Will update in v2
>
>> + u32 num_statistics; /* Number of stats following */
>> + /* zero or more performance matrics */
>> + struct papr_scm_perf_stat scm_statistic[];
>> +} __packed;
>> +
>> /* private struct associated with each region */
>> struct papr_scm_priv {
>> struct platform_device *pdev;
>> @@ -89,6 +107,9 @@ struct papr_scm_priv {
>>
>> /* Health information for the dimm */
>> u64 health_bitmap;
>> +
>> + /* length of the stat buffer as expected by phyp */
>> + size_t len_stat_buffer;
>> };
>>
>> static int drc_pmem_bind(struct papr_scm_priv *p)
>> @@ -194,6 +215,75 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
>> return drc_pmem_bind(p);
>> }
>>
>> +/*
>> + * Query the Dimm performance stats from PHYP and copy them (if returned) to
>> + * provided struct papr_scm_perf_stats instance 'stats' of 'size' in bytes.
>> + * The value of R4 is copied to 'out' if the pointer is provided.
>> + */
>> +static int drc_pmem_query_stats(struct papr_scm_priv *p,
>> + struct papr_scm_perf_stats *buff_stats,
>> + size_t size, unsigned int num_stats,
>> + uint64_t *out)
>> +{
>> + unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> + struct papr_scm_perf_stat *stats;
>> + s64 rc, i;
>> +
>> + /* Setup the out buffer */
>> + if (buff_stats) {
>> + memcpy(buff_stats->eye_catcher,
>> + PAPR_SCM_PERF_STATS_EYECATCHER, 8);
>> + buff_stats->stats_version =
>> + cpu_to_be32(PAPR_SCM_PERF_STATS_VERSION);
>> + buff_stats->num_statistics =
>> + cpu_to_be32(num_stats);
>> + } else {
>> + /* In case of no out buffer ignore the size */
>> + size = 0;
>> + }
>> +
>> + /*
>> + * Do the HCALL asking PHYP for info and if R4 was requested
>> + * return its value in 'out' variable.
>> + */
>> + rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
>> + virt_to_phys(buff_stats), size);
>
> You are calling virt_to_phys(NULL) here when called from
> papr_scm_nvdimm_init()! That can't be right.
Thanks for cathing this. However if the 'size' is '0' the 'buff_stats'
address is ignored by the hypervisor hence this didnt get caught in my
tests. Though CONFIG_DEBUG_VIRTUAL would have caught it early.
>
>> + if (out)
>> + *out = ret[0];
>> +
>> + if (rc == H_PARTIAL) {
>> + dev_err(&p->pdev->dev,
>> + "Unknown performance stats, Err:0x%016lX\n", ret[0]);
>> + return -ENOENT;
>> + } else if (rc != H_SUCCESS) {
>> + dev_err(&p->pdev->dev,
>> + "Failed to query performance stats, Err:%lld\n", rc);
>> + return -ENXIO;
>> + }
>> +
>> + /* Successfully fetched the requested stats from phyp */
>> + if (size != 0) {
>> + buff_stats->num_statistics =
>> + be32_to_cpu(buff_stats->num_statistics);
>> +
>> + /* Transform the stats buffer values from BE to cpu native */
>> + for (i = 0, stats = buff_stats->scm_statistic;
>> + i < buff_stats->num_statistics; ++i) {
>> + stats[i].statistic_value =
>> + be64_to_cpu(stats[i].statistic_value);
>> + }
>> + dev_dbg(&p->pdev->dev,
>> + "Performance stats returned %d stats\n",
>> + buff_stats->num_statistics);
>> + } else {
>> + /* Handle case where stat buffer size was requested */
>> + dev_dbg(&p->pdev->dev,
>> + "Performance stats size %ld\n", ret[0]);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
>> * health information.
>> @@ -631,6 +721,45 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>> return 0;
>> }
>>
>> +static ssize_t perf_stats_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + int index, rc;
>> + struct seq_buf s;
>> + struct papr_scm_perf_stat *stat;
>> + struct papr_scm_perf_stats *stats;
>> + struct nvdimm *dimm = to_nvdimm(dev);
>> + struct papr_scm_priv *p = nvdimm_provider_data(dimm);
>> +
>> + if (!p->len_stat_buffer)
>> + return -ENOENT;
>> +
>> + /* Allocate the buffer for phyp where stats are written */
>> + stats = kzalloc(p->len_stat_buffer, GFP_KERNEL);
>
> I'm concerned that this buffer does not seem to have anything to do with the
> 'num_stats' parameter passed to drc_pmem_query_stats(). Furthermore why is
> num_stats always 0 in those calls?
>
'num_stats == 0' is a special case of the hcall where PHYP returns all
the possible stats in the 'stats' buffer.
>> + if (!stats)
>> + return -ENOMEM;
>> +
>> + /* Ask phyp to return all dimm perf stats */
>> + rc = drc_pmem_query_stats(p, stats, p->len_stat_buffer, 0, NULL);
>> + if (!rc) {
>> + /*
>> + * Go through the returned output buffer and print stats and
>> + * values. Since statistic_id is essentially a char string of
>> + * 8 bytes, simply use the string format specifier to print it.
>> + */
>> + seq_buf_init(&s, buf, PAGE_SIZE);
>> + for (index = 0, stat = stats->scm_statistic;
>> + index < stats->num_statistics; ++index, ++stat) {
>> + seq_buf_printf(&s, "%.8s = 0x%016llX\n",
>> + stat->statistic_id, stat->statistic_value);
>> + }
>> + }
>> +
>> + kfree(stats);
>> + return rc ? rc : seq_buf_used(&s);
>> +}
>> +DEVICE_ATTR_RO(perf_stats);
>> +
>> static ssize_t flags_show(struct device *dev,
>> struct device_attribute *attr, char *buf)
>> {
>> @@ -676,6 +805,7 @@ DEVICE_ATTR_RO(flags);
>> /* papr_scm specific dimm attributes */
>> static struct attribute *papr_nd_attributes[] = {
>> &dev_attr_flags.attr,
>> + &dev_attr_perf_stats.attr,
>> NULL,
>> };
>>
>> @@ -696,6 +826,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>> struct nd_region_desc ndr_desc;
>> unsigned long dimm_flags;
>> int target_nid, online_nid;
>> + u64 stat_size;
>>
>> p->bus_desc.ndctl = papr_scm_ndctl;
>> p->bus_desc.module = THIS_MODULE;
>> @@ -759,6 +890,14 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>> dev_info(dev, "Region registered with target node %d and online node %d",
>> target_nid, online_nid);
>>
>> + /* Try retriving the stat buffer and see if its supported */
>> + if (!drc_pmem_query_stats(p, NULL, 0, 0, &stat_size)) {
>> + p->len_stat_buffer = (size_t)stat_size;
>> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
>> + p->len_stat_buffer);
>> + } else {
>> + dev_info(&p->pdev->dev, "Limited dimm stat info available\n");
>
> Do we really need this print?
nvdimm performance stats can be selectively turned on/off from the
hypervisor management console hence this info message is more like a
warning indicating that extended dimm stat info like 'fuel_gauge' is not
available.
>
> Ira
>
>> + }
>> return 0;
>>
>> err: nvdimm_bus_unregister(p->bus);
>> --
>> 2.26.2
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
From: Vaibhav Jain @ 2020-06-24 14:03 UTC (permalink / raw)
To: Ira Weiny; +Cc: Aneesh Kumar K . V, linuxppc-dev, linux-nvdimm
In-Reply-To: <20200623191410.GH3910394@iweiny-DESK2.sc.intel.com>
Thanks for reviewing this patch Ira,
My responses below:
Ira Weiny <ira.weiny@intel.com> writes:
[snip]
>> +static int papr_pdsm_fuel_gauge(struct papr_scm_priv *p,
>> + union nd_pdsm_payload *payload)
>> +{
>> + int rc, size;
>> + struct papr_scm_perf_stat *stat;
>> + struct papr_scm_perf_stats *stats;
>> +
>> + /* Silently fail if fetching performance metrics isn't supported */
>> + if (!p->len_stat_buffer)
>> + return 0;
>> +
>> + /* Allocate request buffer enough to hold single performance stat */
>> + size = sizeof(struct papr_scm_perf_stats) +
>> + sizeof(struct papr_scm_perf_stat);
>> +
>> + stats = kzalloc(size, GFP_KERNEL);
>> + if (!stats)
>> + return -ENOMEM;
>> +
>> + stat = &stats->scm_statistic[0];
>> + memcpy(&stat->statistic_id, "MemLife ", sizeof(stat->statistic_id));
>> + stat->statistic_value = 0;
>> +
>> + /* Fetch the fuel gauge and populate it in payload */
>> + rc = drc_pmem_query_stats(p, stats, size, 1, NULL);
>> + if (!rc) {
>
> Always best to except the error case...
>
> if (rc) {
> ... print debuging from below...
> goto free_stats;
> }
>
Sure, I don't feel strongly about it. Will update this in v2.
>> + dev_dbg(&p->pdev->dev,
>> + "Fetched fuel-gauge %llu", stat->statistic_value);
>> + payload->health.extension_flags |=
>> + PDSM_DIMM_HEALTH_RUN_GAUGE_VALID;
>> + payload->health.dimm_fuel_gauge = stat->statistic_value;
>> +
>> + rc = sizeof(struct nd_papr_pdsm_health);
>> + }
>> +
>
> free_stats:
>
>> + kfree(stats);
>> + return rc;
>> +}
>> +
>> /* Fetch the DIMM health info and populate it in provided package. */
>> static int papr_pdsm_health(struct papr_scm_priv *p,
>> union nd_pdsm_payload *payload)
>> @@ -546,6 +585,14 @@ static int papr_pdsm_health(struct papr_scm_priv *p,
>>
>> /* struct populated hence can release the mutex now */
>> mutex_unlock(&p->health_mutex);
>> +
>> + /* Populate the fuel gauge meter in the payload */
>> + rc = papr_pdsm_fuel_gauge(p, payload);
>> +
>> + /* Error fetching fuel gauge is not fatal */
>> + if (rc < 0)
>> + dev_dbg(&p->pdev->dev, "Err(%d) fetching fuel gauge\n", rc);
>
> Why even return an error? Just have *_fuel_guage() the print the debugging and
> return void.
>
papr_pdsm_fuel_gauge uses the same signature as other PDSM service
functions as described in pdsm_cmd_desc.service callback. Hence designed
the function signature as such.
>> +
>> rc = sizeof(struct nd_papr_pdsm_health);
>
> You just override rc here anyway...
>
> Ira
>
>>
>> out:
>> --
>> 2.26.2
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
--
Cheers
~ Vaibhav
^ permalink raw reply
* [PATCH] powerpc/pseries: Use doorbells even if XIVE is available
From: Nicholas Piggin @ 2020-06-24 13:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, kvm-ppc, Nicholas Piggin
KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs. An
option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/smp.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..a737a2f87c67 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,14 @@ static int pseries_smp_prepare_cpu(int cpu)
return 0;
}
+static void (*cause_ipi_offcore)(int cpu) __ro_after_init;
+
static void smp_pseries_cause_ipi(int cpu)
{
- /* POWER9 should not use this handler */
if (doorbell_try_core_ipi(cpu))
return;
- icp_ops->cause_ipi(cpu);
+ cause_ipi_offcore(cpu);
}
static int pseries_cause_nmi_ipi(int cpu)
@@ -222,10 +223,7 @@ static __init void pSeries_smp_probe_xics(void)
{
xics_smp_probe();
- if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
- smp_ops->cause_ipi = smp_pseries_cause_ipi;
- else
- smp_ops->cause_ipi = icp_ops->cause_ipi;
+ smp_ops->cause_ipi = icp_ops->cause_ipi;
}
static __init void pSeries_smp_probe(void)
@@ -238,6 +236,18 @@ static __init void pSeries_smp_probe(void)
xive_smp_probe();
else
pSeries_smp_probe_xics();
+
+ /*
+ * KVM emulates doorbells by reading the instruction, which
+ * can't be done if the guest is secure. If a secure guest
+ * runs under PowerVM, it could use msgsndp but would need a
+ * way to distinguish.
+ */
+ if (cpu_has_feature(CPU_FTR_DBELL) &&
+ cpu_has_feature(CPU_FTR_SMT) && !is_secure_guest()) {
+ cause_ipi_offcore = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = smp_pseries_cause_ipi;
+ }
}
static struct smp_ops_t pseries_smp_ops = {
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] cpufreq: Specify default governor on command line
From: Rafael J. Wysocki @ 2020-06-24 12:51 UTC (permalink / raw)
To: Viresh Kumar
Cc: Juri Lelli, Cc: Android Kernel, Vincent Guittot, Arnd Bergmann,
Rafael J. Wysocki, Peter Zijlstra, Linux PM, Quentin Perret,
Rafael J. Wysocki, Linux Kernel Mailing List, Ingo Molnar,
Paul Mackerras, linuxppc-dev, adharmap, Todd Kjos
In-Reply-To: <20200624055023.xofefhohf7wifme5@vireshk-i7>
On Wed, Jun 24, 2020 at 7:50 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 23-06-20, 15:21, Quentin Perret wrote:
> > Currently, the only way to specify the default CPUfreq governor is via
> > Kconfig options, which suits users who can build the kernel themselves
> > perfectly.
> >
> > However, for those who use a distro-like kernel (such as Android, with
> > the Generic Kernel Image project), the only way to use a different
> > default is to boot to userspace, and to then switch using the sysfs
> > interface. Being able to specify the default governor on the command
> > line, like is the case for cpuidle, would enable those users to specify
> > their governor of choice earlier on, and to simplify slighlty the
> > userspace boot procedure.
> >
> > To support this use-case, add a kernel command line parameter enabling
> > to specify a default governor for CPUfreq, which takes precedence over
> > the builtin default.
> >
> > This implementation has one notable limitation: the default governor
> > must be registered before the driver. This is solved for builtin
> > governors and drivers using appropriate *_initcall() functions. And in
> > the modular case, this must be reflected as a constraint on the module
> > loading order.
> >
> > Signed-off-by: Quentin Perret <qperret@google.com>
> > ---
> > .../admin-guide/kernel-parameters.txt | 5 ++++
> > Documentation/admin-guide/pm/cpufreq.rst | 6 ++---
> > drivers/cpufreq/cpufreq.c | 23 +++++++++++++++----
> > 3 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index fb95fad81c79..5fd3c9f187eb 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -703,6 +703,11 @@
> > cpufreq.off=1 [CPU_FREQ]
> > disable the cpufreq sub-system
> >
> > + cpufreq.default_governor=
> > + [CPU_FREQ] Name of the default cpufreq governor to use.
> > + This governor must be registered in the kernel before
> > + the cpufreq driver probes.
> > +
> > cpu_init_udelay=N
> > [X86] Delay for N microsec between assert and de-assert
> > of APIC INIT to start processors. This delay occurs
> > diff --git a/Documentation/admin-guide/pm/cpufreq.rst b/Documentation/admin-guide/pm/cpufreq.rst
> > index 0c74a7784964..368e612145d2 100644
> > --- a/Documentation/admin-guide/pm/cpufreq.rst
> > +++ b/Documentation/admin-guide/pm/cpufreq.rst
> > @@ -147,9 +147,9 @@ CPUs in it.
> >
> > The next major initialization step for a new policy object is to attach a
> > scaling governor to it (to begin with, that is the default scaling governor
> > -determined by the kernel configuration, but it may be changed later
> > -via ``sysfs``). First, a pointer to the new policy object is passed to the
> > -governor's ``->init()`` callback which is expected to initialize all of the
> > +determined by the kernel command line or configuration, but it may be changed
> > +later via ``sysfs``). First, a pointer to the new policy object is passed to
> > +the governor's ``->init()`` callback which is expected to initialize all of the
> > data structures necessary to handle the given policy and, possibly, to add
> > a governor ``sysfs`` interface to it. Next, the governor is started by
> > invoking its ``->start()`` callback.
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 0128de3603df..4b1a5c0173cf 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -50,6 +50,9 @@ static LIST_HEAD(cpufreq_governor_list);
> > #define for_each_governor(__governor) \
> > list_for_each_entry(__governor, &cpufreq_governor_list, governor_list)
> >
> > +static char cpufreq_param_governor[CPUFREQ_NAME_LEN];
> > +static struct cpufreq_governor *default_governor;
> > +
> > /**
> > * The "cpufreq driver" - the arch- or hardware-dependent low
> > * level driver of CPUFreq support, and its spinlock. This lock
> > @@ -1055,7 +1058,6 @@ __weak struct cpufreq_governor *cpufreq_default_governor(void)
> >
> > static int cpufreq_init_policy(struct cpufreq_policy *policy)
> > {
> > - struct cpufreq_governor *def_gov = cpufreq_default_governor();
> > struct cpufreq_governor *gov = NULL;
> > unsigned int pol = CPUFREQ_POLICY_UNKNOWN;
> >
> > @@ -1065,8 +1067,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
> > if (gov) {
> > pr_debug("Restoring governor %s for cpu %d\n",
> > policy->governor->name, policy->cpu);
> > - } else if (def_gov) {
> > - gov = def_gov;
> > + } else if (default_governor) {
> > + gov = default_governor;
> > } else {
> > return -ENODATA;
> > }
> > @@ -1074,8 +1076,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
> > /* Use the default policy if there is no last_policy. */
> > if (policy->last_policy) {
> > pol = policy->last_policy;
> > - } else if (def_gov) {
> > - pol = cpufreq_parse_policy(def_gov->name);
> > + } else if (default_governor) {
> > + pol = cpufreq_parse_policy(default_governor->name);
> > /*
> > * In case the default governor is neiter "performance"
> > * nor "powersave", fall back to the initial policy
> > @@ -2320,6 +2322,9 @@ int cpufreq_register_governor(struct cpufreq_governor *governor)
> > list_add(&governor->governor_list, &cpufreq_governor_list);
> > }
> >
> > + if (!strncasecmp(cpufreq_param_governor, governor->name, CPUFREQ_NAME_LEN))
> > + default_governor = governor;
> > +
> > mutex_unlock(&cpufreq_governor_mutex);
> > return err;
> > }
> > @@ -2348,6 +2353,8 @@ void cpufreq_unregister_governor(struct cpufreq_governor *governor)
> >
> > mutex_lock(&cpufreq_governor_mutex);
> > list_del(&governor->governor_list);
> > + if (governor == default_governor)
> > + default_governor = cpufreq_default_governor();
> > mutex_unlock(&cpufreq_governor_mutex);
> > }
> > EXPORT_SYMBOL_GPL(cpufreq_unregister_governor);
> > @@ -2789,7 +2796,13 @@ static int __init cpufreq_core_init(void)
> > cpufreq_global_kobject = kobject_create_and_add("cpufreq", &cpu_subsys.dev_root->kobj);
> > BUG_ON(!cpufreq_global_kobject);
> >
> > + mutex_lock(&cpufreq_governor_mutex);
> > + if (!default_governor)
> > + default_governor = cpufreq_default_governor();
> > + mutex_unlock(&cpufreq_governor_mutex);
>
> I don't think locking is required here at core-initcall level.
It isn't necessary AFAICS, but it may as well be regarded as
annotation (kind of instead of having a comment explaining why it need
not be used).
^ permalink raw reply
* Re: [PATCH v4 7/8] lockdep: Change hardirq{s_enabled,_context} to per-cpu variables
From: Peter Zijlstra @ 2020-06-24 12:31 UTC (permalink / raw)
To: Marco Elver
Cc: linux-s390, linuxppc-dev, Paul E. McKenney, bigeasy,
the arch/x86 maintainers, heiko.carstens, LKML, Steven Rostedt,
David S. Miller, Ahmed S. Darwish, sparclinux, linux,
Thomas Gleixner, Will Deacon, Ingo Molnar
In-Reply-To: <CANpmjNMj8FZuBrZsH62V3bZEhFvT2zXwLusVOLwNuH_-kLhp2g@mail.gmail.com>
On Wed, Jun 24, 2020 at 12:17:56PM +0200, Marco Elver wrote:
> On Wed, 24 Jun 2020 at 11:01, Peter Zijlstra <peterz@infradead.org> wrote:
> > And I figured a quick way to get rid of that would be something like the
> > below, seeing how volatile gets auto annotated... but that doesn't seem
> > to actually work.
> >
> > What am I missing?
>
> There's one more in include/linux/rcupdate.h. I suggested this at some point:
>
> https://lore.kernel.org/lkml/20200220213317.GA35033@google.com/
>
> To avoid volatiles as I don't think they are needed here.
Urgghh.. local_t is very expensive for this. The current code is
actually fine, even on load-store architectures. Using local_t will only
result in it being more expensive for no gain.
I'll go put data_race() around it.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox