From 40723419f4079d0c7de98d0f3149db915557b55a Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Mon, 26 Oct 2020 11:49:41 +0000 Subject: [PATCH 01/29] kselftest: Enable vDSO test on non x86 platforms Currently the vDSO tests are built only on x86 platforms and cannot be cross compiled. Enable vDSO TARGET for all the platforms. Future patches will extend the tests. Cc: Shuah Khan Signed-off-by: Vincenzo Frascino Acked-by: Thomas Gleixner Signed-off-by: Shuah Khan --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/vDSO/Makefile | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index d9c283503159..2dd60070d775 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -65,6 +65,7 @@ endif TARGETS += tmpfs TARGETS += tpm2 TARGETS += user +TARGETS += vDSO TARGETS += vm TARGETS += x86 TARGETS += zram diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 0069f2f83f86..47031f3c5f3a 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -9,7 +9,6 @@ ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif -ifndef CROSS_COMPILE CFLAGS := -std=gnu99 CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector ifeq ($(CONFIG_X86_32),y) @@ -24,4 +23,3 @@ $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c vdso_standalone_test_x86.c parse_vdso.c \ -o $@ -endif From 693f5ca08ca0767b407b7ca634dbf1b783676ec3 Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Mon, 26 Oct 2020 11:49:42 +0000 Subject: [PATCH 02/29] kselftest: Extend vDSO selftest The current version of the multiarch vDSO selftest verifies only gettimeofday. Extend the vDSO selftest to the other library functions: - time - clock_getres - clock_gettime The extension has been used to verify the unified vdso library on the supported architectures. Cc: Shuah Khan Signed-off-by: Vincenzo Frascino Acked-by: Thomas Gleixner Signed-off-by: Shuah Khan --- tools/testing/selftests/vDSO/Makefile | 2 + tools/testing/selftests/vDSO/vdso_config.h | 90 +++++++ tools/testing/selftests/vDSO/vdso_test_abi.c | 244 +++++++++++++++++++ 3 files changed, 336 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_config.h create mode 100644 tools/testing/selftests/vDSO/vdso_test_abi.c diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 47031f3c5f3a..2ac7448bd414 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu +TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +19,7 @@ endif all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c +$(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_config.h b/tools/testing/selftests/vDSO/vdso_config.h new file mode 100644 index 000000000000..eeb725df6045 --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_config.h @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * vdso_config.h: Configuration options for vDSO tests. + * Copyright (c) 2019 Arm Ltd. + */ +#ifndef __VDSO_CONFIG_H__ +#define __VDSO_CONFIG_H__ + +/* + * Each architecture exports its vDSO implementation with different names + * and a different version from the others, so we need to handle it as a + * special case. + */ +#if defined(__arm__) +#define VDSO_VERSION 0 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#elif defined(__aarch64__) +#define VDSO_VERSION 3 +#define VDSO_NAMES 0 +#elif defined(__powerpc__) +#define VDSO_VERSION 1 +#define VDSO_NAMES 0 +#define VDSO_32BIT 1 +#elif defined(__powerpc64__) +#define VDSO_VERSION 1 +#define VDSO_NAMES 0 +#elif defined (__s390__) +#define VDSO_VERSION 2 +#define VDSO_NAMES 0 +#define VDSO_32BIT 1 +#elif defined (__s390X__) +#define VDSO_VERSION 2 +#define VDSO_NAMES 0 +#elif defined(__mips__) +#define VDSO_VERSION 0 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#elif defined(__sparc__) +#define VDSO_VERSION 0 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#elif defined(__i386__) +#define VDSO_VERSION 0 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#elif defined(__x86_64__) +#define VDSO_VERSION 0 +#define VDSO_NAMES 1 +#elif defined(__riscv__) +#define VDSO_VERSION 5 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#else /* nds32 */ +#define VDSO_VERSION 4 +#define VDSO_NAMES 1 +#define VDSO_32BIT 1 +#endif + +static const char *versions[6] = { + "LINUX_2.6", + "LINUX_2.6.15", + "LINUX_2.6.29", + "LINUX_2.6.39", + "LINUX_4", + "LINUX_4.15", +}; + +static const char *names[2][5] = { + { + "__kernel_gettimeofday", + "__kernel_clock_gettime", + "__kernel_time", + "__kernel_clock_getres", +#if defined(VDSO_32BIT) + "__kernel_clock_gettime64", +#endif + }, + { + "__vdso_gettimeofday", + "__vdso_clock_gettime", + "__vdso_time", + "__vdso_clock_getres", +#if defined(VDSO_32BIT) + "__vdso_clock_gettime64", +#endif + }, +}; + +#endif /* __VDSO_CONFIG_H__ */ diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c new file mode 100644 index 000000000000..3d603f1394af --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -0,0 +1,244 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * vdso_full_test.c: Sample code to test all the timers. + * Copyright (c) 2019 Arm Ltd. + * + * Compile with: + * gcc -std=gnu99 vdso_full_test.c parse_vdso.c + * + */ + +#include +#include +#include +#include +#include +#include +#define _GNU_SOURCE +#include +#include + +#include "../kselftest.h" +#include "vdso_config.h" + +extern void *vdso_sym(const char *version, const char *name); +extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); +extern void vdso_init_from_auxv(void *auxv); + +static const char *version; +static const char **name; + +typedef long (*vdso_gettimeofday_t)(struct timeval *tv, struct timezone *tz); +typedef long (*vdso_clock_gettime_t)(clockid_t clk_id, struct timespec *ts); +typedef long (*vdso_clock_getres_t)(clockid_t clk_id, struct timespec *ts); +typedef time_t (*vdso_time_t)(time_t *t); + +static int vdso_test_gettimeofday(void) +{ + /* Find gettimeofday. */ + vdso_gettimeofday_t vdso_gettimeofday = + (vdso_gettimeofday_t)vdso_sym(version, name[0]); + + if (!vdso_gettimeofday) { + printf("Could not find %s\n", name[0]); + return KSFT_SKIP; + } + + struct timeval tv; + long ret = vdso_gettimeofday(&tv, 0); + + if (ret == 0) { + printf("The time is %lld.%06lld\n", + (long long)tv.tv_sec, (long long)tv.tv_usec); + } else { + printf("%s failed\n", name[0]); + return KSFT_FAIL; + } + + return KSFT_PASS; +} + +static int vdso_test_clock_gettime(clockid_t clk_id) +{ + /* Find clock_gettime. */ + vdso_clock_gettime_t vdso_clock_gettime = + (vdso_clock_gettime_t)vdso_sym(version, name[1]); + + if (!vdso_clock_gettime) { + printf("Could not find %s\n", name[1]); + return KSFT_SKIP; + } + + struct timespec ts; + long ret = vdso_clock_gettime(clk_id, &ts); + + if (ret == 0) { + printf("The time is %lld.%06lld\n", + (long long)ts.tv_sec, (long long)ts.tv_nsec); + } else { + printf("%s failed\n", name[1]); + return KSFT_FAIL; + } + + return KSFT_PASS; +} + +static int vdso_test_time(void) +{ + /* Find time. */ + vdso_time_t vdso_time = + (vdso_time_t)vdso_sym(version, name[2]); + + if (!vdso_time) { + printf("Could not find %s\n", name[2]); + return KSFT_SKIP; + } + + long ret = vdso_time(NULL); + + if (ret > 0) { + printf("The time in hours since January 1, 1970 is %lld\n", + (long long)(ret / 3600)); + } else { + printf("%s failed\n", name[2]); + return KSFT_FAIL; + } + + return KSFT_PASS; +} + +static int vdso_test_clock_getres(clockid_t clk_id) +{ + /* Find clock_getres. */ + vdso_clock_getres_t vdso_clock_getres = + (vdso_clock_getres_t)vdso_sym(version, name[3]); + + if (!vdso_clock_getres) { + printf("Could not find %s\n", name[3]); + return KSFT_SKIP; + } + + struct timespec ts, sys_ts; + long ret = vdso_clock_getres(clk_id, &ts); + + if (ret == 0) { + printf("The resolution is %lld %lld\n", + (long long)ts.tv_sec, (long long)ts.tv_nsec); + } else { + printf("%s failed\n", name[3]); + return KSFT_FAIL; + } + + ret = syscall(SYS_clock_getres, clk_id, &sys_ts); + + if ((sys_ts.tv_sec != ts.tv_sec) || (sys_ts.tv_nsec != ts.tv_nsec)) { + printf("%s failed\n", name[3]); + return KSFT_FAIL; + } + + return KSFT_PASS; +} + +const char *vdso_clock_name[12] = { + "CLOCK_REALTIME", + "CLOCK_MONOTONIC", + "CLOCK_PROCESS_CPUTIME_ID", + "CLOCK_THREAD_CPUTIME_ID", + "CLOCK_MONOTONIC_RAW", + "CLOCK_REALTIME_COARSE", + "CLOCK_MONOTONIC_COARSE", + "CLOCK_BOOTTIME", + "CLOCK_REALTIME_ALARM", + "CLOCK_BOOTTIME_ALARM", + "CLOCK_SGI_CYCLE", + "CLOCK_TAI", +}; + +/* + * This function calls vdso_test_clock_gettime and vdso_test_clock_getres + * with different values for clock_id. + */ +static inline int vdso_test_clock(clockid_t clock_id) +{ + int ret0, ret1; + + ret0 = vdso_test_clock_gettime(clock_id); + /* A skipped test is considered passed */ + if (ret0 == KSFT_SKIP) + ret0 = KSFT_PASS; + + ret1 = vdso_test_clock_getres(clock_id); + /* A skipped test is considered passed */ + if (ret1 == KSFT_SKIP) + ret1 = KSFT_PASS; + + ret0 += ret1; + + printf("clock_id: %s", vdso_clock_name[clock_id]); + + if (ret0 > 0) + printf(" [FAIL]\n"); + else + printf(" [PASS]\n"); + + return ret0; +} + +int main(int argc, char **argv) +{ + unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + int ret; + + if (!sysinfo_ehdr) { + printf("AT_SYSINFO_EHDR is not present!\n"); + return KSFT_SKIP; + } + + version = versions[VDSO_VERSION]; + name = (const char **)&names[VDSO_NAMES]; + + printf("[vDSO kselftest] VDSO_VERSION: %s\n", version); + + vdso_init_from_sysinfo_ehdr(getauxval(AT_SYSINFO_EHDR)); + + ret = vdso_test_gettimeofday(); + +#if _POSIX_TIMERS > 0 + +#ifdef CLOCK_REALTIME + ret += vdso_test_clock(CLOCK_REALTIME); +#endif + +#ifdef CLOCK_BOOTTIME + ret += vdso_test_clock(CLOCK_BOOTTIME); +#endif + +#ifdef CLOCK_TAI + ret += vdso_test_clock(CLOCK_TAI); +#endif + +#ifdef CLOCK_REALTIME_COARSE + ret += vdso_test_clock(CLOCK_REALTIME_COARSE); +#endif + +#ifdef CLOCK_MONOTONIC + ret += vdso_test_clock(CLOCK_MONOTONIC); +#endif + +#ifdef CLOCK_MONOTONIC_RAW + ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); +#endif + +#ifdef CLOCK_MONOTONIC_COARSE + ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); +#endif + +#endif + + ret += vdso_test_time(); + + if (ret > 0) + return KSFT_FAIL; + + return KSFT_PASS; +} From 03f55c7952c92d8577d6e9bc695f3fd20032cfd9 Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Mon, 26 Oct 2020 11:49:43 +0000 Subject: [PATCH 03/29] kselftest: Extend vDSO selftest to clock_getres The current version of the multiarch vDSO selftest verifies only gettimeofday. Extend the vDSO selftest to clock_getres, to verify that the syscall and the vDSO library function return the same information. The extension has been used to verify the hrtimer_resoltion fix. Cc: Shuah Khan Signed-off-by: Vincenzo Frascino Acked-by: Thomas Gleixner Signed-off-by: Shuah Khan --- tools/testing/selftests/vDSO/Makefile | 2 + .../selftests/vDSO/vdso_test_clock_getres.c | 124 ++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 tools/testing/selftests/vDSO/vdso_test_clock_getres.c diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 2ac7448bd414..375d80c2bff5 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -6,6 +6,7 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi +TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -20,6 +21,7 @@ all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c +$(OUTPUT)/vdso_test_clock_getres: vdso_test_clock_getres.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c new file mode 100644 index 000000000000..15dcee16ff72 --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note +/* + * vdso_clock_getres.c: Sample code to test clock_getres. + * Copyright (c) 2019 Arm Ltd. + * + * Compile with: + * gcc -std=gnu99 vdso_clock_getres.c + * + * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit), + * Power (32-bit and 64-bit), S390x (32-bit and 64-bit). + * Might work on other architectures. + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../kselftest.h" + +static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts) +{ + long ret; + + ret = syscall(SYS_clock_getres, _clkid, _ts); + + return ret; +} + +const char *vdso_clock_name[12] = { + "CLOCK_REALTIME", + "CLOCK_MONOTONIC", + "CLOCK_PROCESS_CPUTIME_ID", + "CLOCK_THREAD_CPUTIME_ID", + "CLOCK_MONOTONIC_RAW", + "CLOCK_REALTIME_COARSE", + "CLOCK_MONOTONIC_COARSE", + "CLOCK_BOOTTIME", + "CLOCK_REALTIME_ALARM", + "CLOCK_BOOTTIME_ALARM", + "CLOCK_SGI_CYCLE", + "CLOCK_TAI", +}; + +/* + * This function calls clock_getres in vdso and by system call + * with different values for clock_id. + * + * Example of output: + * + * clock_id: CLOCK_REALTIME [PASS] + * clock_id: CLOCK_BOOTTIME [PASS] + * clock_id: CLOCK_TAI [PASS] + * clock_id: CLOCK_REALTIME_COARSE [PASS] + * clock_id: CLOCK_MONOTONIC [PASS] + * clock_id: CLOCK_MONOTONIC_RAW [PASS] + * clock_id: CLOCK_MONOTONIC_COARSE [PASS] + */ +static inline int vdso_test_clock(unsigned int clock_id) +{ + struct timespec x, y; + + printf("clock_id: %s", vdso_clock_name[clock_id]); + clock_getres(clock_id, &x); + syscall_clock_getres(clock_id, &y); + + if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) { + printf(" [FAIL]\n"); + return KSFT_FAIL; + } + + printf(" [PASS]\n"); + return KSFT_PASS; +} + +int main(int argc, char **argv) +{ + int ret; + +#if _POSIX_TIMERS > 0 + +#ifdef CLOCK_REALTIME + ret = vdso_test_clock(CLOCK_REALTIME); +#endif + +#ifdef CLOCK_BOOTTIME + ret += vdso_test_clock(CLOCK_BOOTTIME); +#endif + +#ifdef CLOCK_TAI + ret += vdso_test_clock(CLOCK_TAI); +#endif + +#ifdef CLOCK_REALTIME_COARSE + ret += vdso_test_clock(CLOCK_REALTIME_COARSE); +#endif + +#ifdef CLOCK_MONOTONIC + ret += vdso_test_clock(CLOCK_MONOTONIC); +#endif + +#ifdef CLOCK_MONOTONIC_RAW + ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); +#endif + +#ifdef CLOCK_MONOTONIC_COARSE + ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); +#endif + +#endif + if (ret > 0) + return KSFT_FAIL; + + return KSFT_PASS; +} From c7e5789b24d36dd5dddd36ea2b99280a606cac42 Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Mon, 26 Oct 2020 11:49:44 +0000 Subject: [PATCH 04/29] kselftest: Move test_vdso to the vDSO test suite Move test_vdso from x86 to the vDSO test suite. Suggested-by: Andy Lutomirski Cc: Shuah Khan Signed-off-by: Vincenzo Frascino Acked-by: Thomas Gleixner Signed-off-by: Shuah Khan --- tools/testing/selftests/vDSO/Makefile | 10 ++++++++-- .../{x86/test_vdso.c => vDSO/vdso_test_correctness.c} | 0 tools/testing/selftests/x86/Makefile | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) rename tools/testing/selftests/{x86/test_vdso.c => vDSO/vdso_test_correctness.c} (100%) diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 375d80c2bff5..d53a4d8008f9 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -7,12 +7,14 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres -ifeq ($(ARCH),x86) +ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64)) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif +TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness CFLAGS := -std=gnu99 CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector +LDFLAGS_vdso_test_correctness := -ldl ifeq ($(CONFIG_X86_32),y) LDLIBS += -lgcc_s endif @@ -26,4 +28,8 @@ $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ -o $@ - +$(OUTPUT)/vdso_test_correctness: vdso_test_correctness.c + $(CC) $(CFLAGS) \ + vdso_test_correctness.c \ + -o $@ \ + $(LDFLAGS_vdso_test_correctness) diff --git a/tools/testing/selftests/x86/test_vdso.c b/tools/testing/selftests/vDSO/vdso_test_correctness.c similarity index 100% rename from tools/testing/selftests/x86/test_vdso.c rename to tools/testing/selftests/vDSO/vdso_test_correctness.c diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 6703c7906b71..333980375bc7 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -12,7 +12,7 @@ CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh $(CC) trivial_program.c -no-pie) TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \ check_initial_reg_state sigreturn iopl ioperm \ - test_vdso test_vsyscall mov_ss_trap \ + test_vsyscall mov_ss_trap \ syscall_arg_fault fsgsbase_restore TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ From b2f1c3db28870d88d1a19aa86a8374e7725d62c5 Mon Sep 17 00:00:00 2001 From: Vincenzo Frascino Date: Mon, 26 Oct 2020 11:49:45 +0000 Subject: [PATCH 05/29] kselftest: Extend vdso correctness test to clock_gettime64 With the release of Linux 5.1 has been added a new syscall, clock_gettime64, that provided a 64 bit time value for a specified clock_ID to make the kernel Y2038 safe on 32 bit architectures. Extend the vdso correctness test to cover the newly exposed vdso function. Cc: Shuah Khan Signed-off-by: Vincenzo Frascino Acked-by: Thomas Gleixner Signed-off-by: Shuah Khan --- tools/testing/selftests/vDSO/vdso_config.h | 4 +- .../selftests/vDSO/vdso_test_correctness.c | 115 +++++++++++++++++- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/vDSO/vdso_config.h b/tools/testing/selftests/vDSO/vdso_config.h index eeb725df6045..6a6fe8d4ff55 100644 --- a/tools/testing/selftests/vDSO/vdso_config.h +++ b/tools/testing/selftests/vDSO/vdso_config.h @@ -66,12 +66,13 @@ static const char *versions[6] = { "LINUX_4.15", }; -static const char *names[2][5] = { +static const char *names[2][6] = { { "__kernel_gettimeofday", "__kernel_clock_gettime", "__kernel_time", "__kernel_clock_getres", + "__kernel_getcpu", #if defined(VDSO_32BIT) "__kernel_clock_gettime64", #endif @@ -81,6 +82,7 @@ static const char *names[2][5] = { "__vdso_clock_gettime", "__vdso_time", "__vdso_clock_getres", + "__vdso_getcpu", #if defined(VDSO_32BIT) "__vdso_clock_gettime64", #endif diff --git a/tools/testing/selftests/vDSO/vdso_test_correctness.c b/tools/testing/selftests/vDSO/vdso_test_correctness.c index 42052db0f870..5029ef9b228c 100644 --- a/tools/testing/selftests/vDSO/vdso_test_correctness.c +++ b/tools/testing/selftests/vDSO/vdso_test_correctness.c @@ -19,6 +19,10 @@ #include #include +#include "vdso_config.h" + +static const char **name; + #ifndef SYS_getcpu # ifdef __x86_64__ # define SYS_getcpu 309 @@ -27,6 +31,17 @@ # endif #endif +#ifndef __NR_clock_gettime64 +#define __NR_clock_gettime64 403 +#endif + +#ifndef __kernel_timespec +struct __kernel_timespec { + long long tv_sec; + long long tv_nsec; +}; +#endif + /* max length of lines in /proc/self/maps - anything longer is skipped here */ #define MAPS_LINE_LEN 128 @@ -36,6 +51,10 @@ typedef int (*vgettime_t)(clockid_t, struct timespec *); vgettime_t vdso_clock_gettime; +typedef int (*vgettime64_t)(clockid_t, struct __kernel_timespec *); + +vgettime64_t vdso_clock_gettime64; + typedef long (*vgtod_t)(struct timeval *tv, struct timezone *tz); vgtod_t vdso_gettimeofday; @@ -99,17 +118,23 @@ static void fill_function_pointers() return; } - vdso_getcpu = (getcpu_t)dlsym(vdso, "__vdso_getcpu"); + vdso_getcpu = (getcpu_t)dlsym(vdso, name[4]); if (!vdso_getcpu) printf("Warning: failed to find getcpu in vDSO\n"); vgetcpu = (getcpu_t) vsyscall_getcpu(); - vdso_clock_gettime = (vgettime_t)dlsym(vdso, "__vdso_clock_gettime"); + vdso_clock_gettime = (vgettime_t)dlsym(vdso, name[1]); if (!vdso_clock_gettime) printf("Warning: failed to find clock_gettime in vDSO\n"); - vdso_gettimeofday = (vgtod_t)dlsym(vdso, "__vdso_gettimeofday"); +#if defined(VDSO_32BIT) + vdso_clock_gettime64 = (vgettime64_t)dlsym(vdso, name[5]); + if (!vdso_clock_gettime64) + printf("Warning: failed to find clock_gettime64 in vDSO\n"); +#endif + + vdso_gettimeofday = (vgtod_t)dlsym(vdso, name[0]); if (!vdso_gettimeofday) printf("Warning: failed to find gettimeofday in vDSO\n"); @@ -126,6 +151,11 @@ static inline int sys_clock_gettime(clockid_t id, struct timespec *ts) return syscall(__NR_clock_gettime, id, ts); } +static inline int sys_clock_gettime64(clockid_t id, struct __kernel_timespec *ts) +{ + return syscall(__NR_clock_gettime64, id, ts); +} + static inline int sys_gettimeofday(struct timeval *tv, struct timezone *tz) { return syscall(__NR_gettimeofday, tv, tz); @@ -191,6 +221,15 @@ static bool ts_leq(const struct timespec *a, const struct timespec *b) return a->tv_nsec <= b->tv_nsec; } +static bool ts64_leq(const struct __kernel_timespec *a, + const struct __kernel_timespec *b) +{ + if (a->tv_sec != b->tv_sec) + return a->tv_sec < b->tv_sec; + else + return a->tv_nsec <= b->tv_nsec; +} + static bool tv_leq(const struct timeval *a, const struct timeval *b) { if (a->tv_sec != b->tv_sec) @@ -254,7 +293,10 @@ static void test_one_clock_gettime(int clock, const char *name) if (!ts_leq(&start, &vdso) || !ts_leq(&vdso, &end)) { printf("[FAIL]\tTimes are out of sequence\n"); nerrs++; + return; } + + printf("[OK]\tTest Passed.\n"); } static void test_clock_gettime(void) @@ -275,6 +317,70 @@ static void test_clock_gettime(void) test_one_clock_gettime(INT_MAX, "invalid"); } +static void test_one_clock_gettime64(int clock, const char *name) +{ + struct __kernel_timespec start, vdso, end; + int vdso_ret, end_ret; + + printf("[RUN]\tTesting clock_gettime64 for clock %s (%d)...\n", name, clock); + + if (sys_clock_gettime64(clock, &start) < 0) { + if (errno == EINVAL) { + vdso_ret = vdso_clock_gettime64(clock, &vdso); + if (vdso_ret == -EINVAL) { + printf("[OK]\tNo such clock.\n"); + } else { + printf("[FAIL]\tNo such clock, but __vdso_clock_gettime64 returned %d\n", vdso_ret); + nerrs++; + } + } else { + printf("[WARN]\t clock_gettime64(%d) syscall returned error %d\n", clock, errno); + } + return; + } + + vdso_ret = vdso_clock_gettime64(clock, &vdso); + end_ret = sys_clock_gettime64(clock, &end); + + if (vdso_ret != 0 || end_ret != 0) { + printf("[FAIL]\tvDSO returned %d, syscall errno=%d\n", + vdso_ret, errno); + nerrs++; + return; + } + + printf("\t%llu.%09ld %llu.%09ld %llu.%09ld\n", + (unsigned long long)start.tv_sec, start.tv_nsec, + (unsigned long long)vdso.tv_sec, vdso.tv_nsec, + (unsigned long long)end.tv_sec, end.tv_nsec); + + if (!ts64_leq(&start, &vdso) || !ts64_leq(&vdso, &end)) { + printf("[FAIL]\tTimes are out of sequence\n"); + nerrs++; + return; + } + + printf("[OK]\tTest Passed.\n"); +} + +static void test_clock_gettime64(void) +{ + if (!vdso_clock_gettime64) { + printf("[SKIP]\tNo vDSO, so skipping clock_gettime64() tests\n"); + return; + } + + for (int clock = 0; clock < sizeof(clocknames) / sizeof(clocknames[0]); + clock++) { + test_one_clock_gettime64(clock, clocknames[clock]); + } + + /* Also test some invalid clock ids */ + test_one_clock_gettime64(-1, "invalid"); + test_one_clock_gettime64(INT_MIN, "invalid"); + test_one_clock_gettime64(INT_MAX, "invalid"); +} + static void test_gettimeofday(void) { struct timeval start, vdso, end; @@ -327,9 +433,12 @@ static void test_gettimeofday(void) int main(int argc, char **argv) { + name = (const char **)&names[VDSO_NAMES]; + fill_function_pointers(); test_clock_gettime(); + test_clock_gettime64(); test_gettimeofday(); /* From ff2c395b9257f0e617f9cd212893f3c72c80ee6c Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 4 Nov 2020 21:08:40 +1100 Subject: [PATCH 06/29] selftests/gpio: Use TEST_GEN_PROGS_EXTENDED Use TEST_GEN_PROGS_EXTENDED rather than TEST_PROGS_EXTENDED. That tells the lib.mk logic that the files it references are to be generated by the Makefile. Having done that we don't need to override the all rule. Signed-off-by: Michael Ellerman Signed-off-by: Shuah Khan --- tools/testing/selftests/gpio/Makefile | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile index 32bdc978a711..c85fb5acf5f4 100644 --- a/tools/testing/selftests/gpio/Makefile +++ b/tools/testing/selftests/gpio/Makefile @@ -11,22 +11,20 @@ LDLIBS += $(VAR_LDLIBS) TEST_PROGS := gpio-mockup.sh TEST_FILES := gpio-mockup-sysfs.sh -TEST_PROGS_EXTENDED := gpio-mockup-chardev +TEST_GEN_PROGS_EXTENDED := gpio-mockup-chardev GPIODIR := $(realpath ../../../gpio) GPIOOBJ := gpio-utils.o -all: $(TEST_PROGS_EXTENDED) - override define CLEAN - $(RM) $(TEST_PROGS_EXTENDED) + $(RM) $(TEST_GEN_PROGS_EXTENDED) $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean endef KSFT_KHDR_INSTALL := 1 include ../lib.mk -$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ) +$(TEST_GEN_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ) $(GPIODIR)/$(GPIOOBJ): $(MAKE) OUTPUT=$(GPIODIR)/ -C $(GPIODIR) From 449539da2e237336bc750b41f1736a77f9aca25c Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 4 Nov 2020 21:08:41 +1100 Subject: [PATCH 07/29] selftests/gpio: Move include of lib.mk up Move the include of lib.mk up so that in a subsequent patch we can use OUTPUT, which is initialised by lib.mk, in the definition of the GPIO variables. Signed-off-by: Michael Ellerman Signed-off-by: Shuah Khan --- tools/testing/selftests/gpio/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile index c85fb5acf5f4..615c8a953ade 100644 --- a/tools/testing/selftests/gpio/Makefile +++ b/tools/testing/selftests/gpio/Makefile @@ -13,6 +13,9 @@ TEST_PROGS := gpio-mockup.sh TEST_FILES := gpio-mockup-sysfs.sh TEST_GEN_PROGS_EXTENDED := gpio-mockup-chardev +KSFT_KHDR_INSTALL := 1 +include ../lib.mk + GPIODIR := $(realpath ../../../gpio) GPIOOBJ := gpio-utils.o @@ -21,9 +24,6 @@ override define CLEAN $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean endef -KSFT_KHDR_INSTALL := 1 -include ../lib.mk - $(TEST_GEN_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ) $(GPIODIR)/$(GPIOOBJ): From b68c1c65dec5fb5186ebd33ce52059b4c6db8500 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 4 Nov 2020 21:08:42 +1100 Subject: [PATCH 08/29] selftests/gpio: Fix build when source tree is read only Currently the gpio selftests fail to build if the source tree is read only: make -j 160 -C tools/testing/selftests TARGETS=gpio make[1]: Entering directory '/linux/tools/testing/selftests/gpio' make OUTPUT=/linux/tools/gpio/ -C /linux/tools/gpio make[2]: Entering directory '/linux/tools/gpio' mkdir -p /linux/tools/gpio/include/linux 2>&1 || true ln -sf /linux/tools/gpio/../../include/uapi/linux/gpio.h /linux/tools/gpio/include/linux/gpio.h ln: failed to create symbolic link '/linux/tools/gpio/include/linux/gpio.h': Read-only file system This happens because we ask make to build ../../../gpio (tools/gpio) without pointing OUTPUT away from the source directory. To fix it we create a subdirectory of the existing OUTPUT directory, called tools-gpio, and tell tools/gpio to build in there. Signed-off-by: Michael Ellerman Signed-off-by: Shuah Khan --- tools/testing/selftests/gpio/Makefile | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile index 615c8a953ade..acf4088a9891 100644 --- a/tools/testing/selftests/gpio/Makefile +++ b/tools/testing/selftests/gpio/Makefile @@ -17,14 +17,18 @@ KSFT_KHDR_INSTALL := 1 include ../lib.mk GPIODIR := $(realpath ../../../gpio) -GPIOOBJ := gpio-utils.o +GPIOOUT := $(OUTPUT)/tools-gpio/ +GPIOOBJ := $(GPIOOUT)/gpio-utils.o override define CLEAN $(RM) $(TEST_GEN_PROGS_EXTENDED) - $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean + $(RM) -rf $(GPIOOUT) endef -$(TEST_GEN_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ) +$(TEST_GEN_PROGS_EXTENDED): $(GPIOOBJ) -$(GPIODIR)/$(GPIOOBJ): - $(MAKE) OUTPUT=$(GPIODIR)/ -C $(GPIODIR) +$(GPIOOUT): + mkdir -p $@ + +$(GPIOOBJ): $(GPIOOUT) + $(MAKE) OUTPUT=$(GPIOOUT) -C $(GPIODIR) From 85128c5bcdf9bd9b574d7cbafa49170a39fed2e1 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 4 Nov 2020 21:08:43 +1100 Subject: [PATCH 09/29] selftests/gpio: Add to CLEAN rule rather than overriding Rather than overriding the CLEAN rule we can just append to it. Signed-off-by: Michael Ellerman Signed-off-by: Shuah Khan --- tools/testing/selftests/gpio/Makefile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile index acf4088a9891..41582fe485ee 100644 --- a/tools/testing/selftests/gpio/Makefile +++ b/tools/testing/selftests/gpio/Makefile @@ -20,10 +20,7 @@ GPIODIR := $(realpath ../../../gpio) GPIOOUT := $(OUTPUT)/tools-gpio/ GPIOOBJ := $(GPIOOUT)/gpio-utils.o -override define CLEAN - $(RM) $(TEST_GEN_PROGS_EXTENDED) - $(RM) -rf $(GPIOOUT) -endef +CLEAN += ; $(RM) -rf $(GPIOOUT) $(TEST_GEN_PROGS_EXTENDED): $(GPIOOBJ) From fc4a3a1bf9ad799181e4d4ec9c2598c0405bc27d Mon Sep 17 00:00:00 2001 From: Tommi Rantala Date: Mon, 2 Nov 2020 09:39:18 +0200 Subject: [PATCH 10/29] selftests: intel_pstate: ftime() is deprecated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use clock_gettime() instead of deprecated ftime(). aperf.c: In function ‘main’: aperf.c:58:2: warning: ‘ftime’ is deprecated [-Wdeprecated-declarations] 58 | ftime(&before); | ^~~~~ In file included from aperf.c:9: /usr/include/sys/timeb.h:39:12: note: declared here 39 | extern int ftime (struct timeb *__timebuf) | ^~~~~ Signed-off-by: Tommi Rantala Signed-off-by: Shuah Khan --- tools/testing/selftests/intel_pstate/aperf.c | 22 ++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/intel_pstate/aperf.c b/tools/testing/selftests/intel_pstate/aperf.c index f6cd03a87493..a8acf3996973 100644 --- a/tools/testing/selftests/intel_pstate/aperf.c +++ b/tools/testing/selftests/intel_pstate/aperf.c @@ -10,8 +10,12 @@ #include #include #include +#include #include "../kselftest.h" +#define MSEC_PER_SEC 1000L +#define NSEC_PER_MSEC 1000000L + void usage(char *name) { printf ("Usage: %s cpunum\n", name); } @@ -22,7 +26,7 @@ int main(int argc, char **argv) { long long tsc, old_tsc, new_tsc; long long aperf, old_aperf, new_aperf; long long mperf, old_mperf, new_mperf; - struct timeb before, after; + struct timespec before, after; long long int start, finish, total; cpu_set_t cpuset; @@ -55,7 +59,10 @@ int main(int argc, char **argv) { return 1; } - ftime(&before); + if (clock_gettime(CLOCK_MONOTONIC, &before) < 0) { + perror("clock_gettime"); + return 1; + } pread(fd, &old_tsc, sizeof(old_tsc), 0x10); pread(fd, &old_aperf, sizeof(old_mperf), 0xe7); pread(fd, &old_mperf, sizeof(old_aperf), 0xe8); @@ -64,7 +71,10 @@ int main(int argc, char **argv) { sqrt(i); } - ftime(&after); + if (clock_gettime(CLOCK_MONOTONIC, &after) < 0) { + perror("clock_gettime"); + return 1; + } pread(fd, &new_tsc, sizeof(new_tsc), 0x10); pread(fd, &new_aperf, sizeof(new_mperf), 0xe7); pread(fd, &new_mperf, sizeof(new_aperf), 0xe8); @@ -73,11 +83,11 @@ int main(int argc, char **argv) { aperf = new_aperf-old_aperf; mperf = new_mperf-old_mperf; - start = before.time*1000 + before.millitm; - finish = after.time*1000 + after.millitm; + start = before.tv_sec*MSEC_PER_SEC + before.tv_nsec/NSEC_PER_MSEC; + finish = after.tv_sec*MSEC_PER_SEC + after.tv_nsec/NSEC_PER_MSEC; total = finish - start; - printf("runTime: %4.2f\n", 1.0*total/1000); + printf("runTime: %4.2f\n", 1.0*total/MSEC_PER_SEC); printf("freq: %7.0f\n", tsc / (1.0*aperf / (1.0 * mperf)) / total); return 0; } From 1c49e3783f8899555190a49024ac86d3d76633cd Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 4 Nov 2020 21:03:05 +1100 Subject: [PATCH 11/29] selftests/memfd: Fix implicit declaration warnings The memfd tests emit several warnings: fuse_test.c:261:7: warning: implicit declaration of function 'open' fuse_test.c:67:6: warning: implicit declaration of function 'fcntl' memfd_test.c:397:6: warning: implicit declaration of function 'fallocate' memfd_test.c:64:7: warning: implicit declaration of function 'open' memfd_test.c:90:6: warning: implicit declaration of function 'fcntl' These are all caused by the test not including fcntl.h. Instead of including linux/fcntl.h, include fcntl.h, which should eventually cause the former to be included as well. Signed-off-by: Michael Ellerman Signed-off-by: Shuah Khan --- tools/testing/selftests/memfd/fuse_test.c | 2 +- tools/testing/selftests/memfd/memfd_test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/memfd/fuse_test.c b/tools/testing/selftests/memfd/fuse_test.c index b018e835737d..be675002f918 100644 --- a/tools/testing/selftests/memfd/fuse_test.c +++ b/tools/testing/selftests/memfd/fuse_test.c @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c index 334a7eea2004..74baab83fec3 100644 --- a/tools/testing/selftests/memfd/memfd_test.c +++ b/tools/testing/selftests/memfd/memfd_test.c @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include #include From 82f147944c650a07831c796c398f5c973dbdde79 Mon Sep 17 00:00:00 2001 From: Wang Qing Date: Sat, 7 Nov 2020 17:19:35 +0800 Subject: [PATCH 12/29] tool: selftests: fix spelling typo of 'writting' writting -> writing Signed-off-by: Wang Qing Reviewed-by: Mike Rapoport Signed-off-by: Shuah Khan --- tools/testing/selftests/vm/userfaultfd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c index 9b0912a01777..9132fae7ad65 100644 --- a/tools/testing/selftests/vm/userfaultfd.c +++ b/tools/testing/selftests/vm/userfaultfd.c @@ -894,7 +894,7 @@ static int faulting_process(int signal_test) count_verify[nr]); } /* - * Trigger write protection if there is by writting + * Trigger write protection if there is by writing * the same value back. */ *area_count(area_dst, nr) = count; @@ -922,7 +922,7 @@ static int faulting_process(int signal_test) count_verify[nr]); exit(1); } /* - * Trigger write protection if there is by writting + * Trigger write protection if there is by writing * the same value back. */ *area_count(area_dst, nr) = count; From 93f20eff0cca972d74cb554a2e8b47730228be16 Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Wed, 28 Oct 2020 16:31:14 +0800 Subject: [PATCH 13/29] selftests/run_kselftest.sh: fix dry-run typo Should be -d instead of -n for dry-run. Fixes: 5da1918446a1 ("selftests/run_kselftest.sh: Make each test individually selectable") Signed-off-by: Hangbin Liu Signed-off-by: Shuah Khan --- tools/testing/selftests/run_kselftest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/run_kselftest.sh b/tools/testing/selftests/run_kselftest.sh index 609a4ef9300e..97165a83df63 100755 --- a/tools/testing/selftests/run_kselftest.sh +++ b/tools/testing/selftests/run_kselftest.sh @@ -48,7 +48,7 @@ while true; do -l | --list) echo "$available" exit 0 ;; - -n | --dry-run) + -d | --dry-run) dryrun="echo" shift ;; -h | --help) From c2e46f6b3e3551558d44c4dc518b9667cb0d5f8b Mon Sep 17 00:00:00 2001 From: Sachin Sant Date: Fri, 6 Nov 2020 13:10:06 +0530 Subject: [PATCH 14/29] selftests/cgroup: Fix build on older distros On older distros struct clone_args does not have a cgroup member, leading to build errors: cgroup_util.c: In function 'clone_into_cgroup': cgroup_util.c:343:4: error: 'struct clone_args' has no member named 'cgroup' cgroup_util.c:346:33: error: invalid application of 'sizeof' to incomplete type 'struct clone_args' But the selftests already have a locally defined version of the structure which is up to date, called __clone_args. So use __clone_args which fixes the error. Signed-off-by: Michael Ellerman Signed-off-by: Sachin Sant > Acked-by: Christian Brauner Signed-off-by: Shuah Khan --- tools/testing/selftests/cgroup/cgroup_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c index 05853b0b8831..027014662fb2 100644 --- a/tools/testing/selftests/cgroup/cgroup_util.c +++ b/tools/testing/selftests/cgroup/cgroup_util.c @@ -337,13 +337,13 @@ pid_t clone_into_cgroup(int cgroup_fd) #ifdef CLONE_ARGS_SIZE_VER2 pid_t pid; - struct clone_args args = { + struct __clone_args args = { .flags = CLONE_INTO_CGROUP, .exit_signal = SIGCHLD, .cgroup = cgroup_fd, }; - pid = sys_clone3(&args, sizeof(struct clone_args)); + pid = sys_clone3(&args, sizeof(struct __clone_args)); /* * Verify that this is a genuine test failure: * ENOSYS -> clone3() not available From 584da076866f38ffb952efcc25af039f9551df81 Mon Sep 17 00:00:00 2001 From: Nikolay Borisov Date: Tue, 10 Nov 2020 15:26:49 +0200 Subject: [PATCH 15/29] printk: ringbuffer: Reference text_data_ring directly in callees. A bunch of functions in the new ringbuffer code take both a printk_ringbuffer struct and a separate prb_data_ring. This is a relic from an earlier version of the code when a second data ring was present. Since this is no longer the case remove the extra function argument from: - data_make_reusable() - data_push_tail() - data_alloc() - data_realloc() Signed-off-by: Nikolay Borisov Reviewed-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek --- kernel/printk/printk_ringbuffer.c | 32 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c index 24a960a89aa8..3b24c3aa55f4 100644 --- a/kernel/printk/printk_ringbuffer.c +++ b/kernel/printk/printk_ringbuffer.c @@ -559,11 +559,12 @@ static void desc_make_reusable(struct prb_desc_ring *desc_ring, * on error the caller can re-load the tail lpos to determine the situation. */ static bool data_make_reusable(struct printk_ringbuffer *rb, - struct prb_data_ring *data_ring, unsigned long lpos_begin, unsigned long lpos_end, unsigned long *lpos_out) { + + struct prb_data_ring *data_ring = &rb->text_data_ring; struct prb_desc_ring *desc_ring = &rb->desc_ring; struct prb_data_block *blk; enum desc_state d_state; @@ -625,10 +626,9 @@ static bool data_make_reusable(struct printk_ringbuffer *rb, * descriptors into the reusable state if the tail is pushed beyond * their associated data block. */ -static bool data_push_tail(struct printk_ringbuffer *rb, - struct prb_data_ring *data_ring, - unsigned long lpos) +static bool data_push_tail(struct printk_ringbuffer *rb, unsigned long lpos) { + struct prb_data_ring *data_ring = &rb->text_data_ring; unsigned long tail_lpos_new; unsigned long tail_lpos; unsigned long next_lpos; @@ -669,8 +669,7 @@ static bool data_push_tail(struct printk_ringbuffer *rb, * Make all descriptors reusable that are associated with * data blocks before @lpos. */ - if (!data_make_reusable(rb, data_ring, tail_lpos, lpos, - &next_lpos)) { + if (!data_make_reusable(rb, tail_lpos, lpos, &next_lpos)) { /* * 1. Guarantee the block ID loaded in * data_make_reusable() is performed before @@ -807,7 +806,7 @@ static bool desc_push_tail(struct printk_ringbuffer *rb, * data blocks once their associated descriptor is gone. */ - if (!data_push_tail(rb, &rb->text_data_ring, desc.text_blk_lpos.next)) + if (!data_push_tail(rb, desc.text_blk_lpos.next)) return false; /* @@ -1021,10 +1020,10 @@ static unsigned long get_next_lpos(struct prb_data_ring *data_ring, * if necessary. This function also associates the data block with * a specified descriptor. */ -static char *data_alloc(struct printk_ringbuffer *rb, - struct prb_data_ring *data_ring, unsigned int size, +static char *data_alloc(struct printk_ringbuffer *rb, unsigned int size, struct prb_data_blk_lpos *blk_lpos, unsigned long id) { + struct prb_data_ring *data_ring = &rb->text_data_ring; struct prb_data_block *blk; unsigned long begin_lpos; unsigned long next_lpos; @@ -1043,7 +1042,7 @@ static char *data_alloc(struct printk_ringbuffer *rb, do { next_lpos = get_next_lpos(data_ring, begin_lpos, size); - if (!data_push_tail(rb, data_ring, next_lpos - DATA_SIZE(data_ring))) { + if (!data_push_tail(rb, next_lpos - DATA_SIZE(data_ring))) { /* Failed to allocate, specify a data-less block. */ blk_lpos->begin = FAILED_LPOS; blk_lpos->next = FAILED_LPOS; @@ -1102,10 +1101,10 @@ static char *data_alloc(struct printk_ringbuffer *rb, * Return a pointer to the beginning of the entire data buffer or NULL on * failure. */ -static char *data_realloc(struct printk_ringbuffer *rb, - struct prb_data_ring *data_ring, unsigned int size, +static char *data_realloc(struct printk_ringbuffer *rb, unsigned int size, struct prb_data_blk_lpos *blk_lpos, unsigned long id) { + struct prb_data_ring *data_ring = &rb->text_data_ring; struct prb_data_block *blk; unsigned long head_lpos; unsigned long next_lpos; @@ -1132,7 +1131,7 @@ static char *data_realloc(struct printk_ringbuffer *rb, return &blk->data[0]; } - if (!data_push_tail(rb, data_ring, next_lpos - DATA_SIZE(data_ring))) + if (!data_push_tail(rb, next_lpos - DATA_SIZE(data_ring))) return NULL; /* The memory barrier involvement is the same as data_alloc:A. */ @@ -1397,7 +1396,7 @@ bool prb_reserve_in_last(struct prb_reserved_entry *e, struct printk_ringbuffer if (r->text_buf_size > max_size) goto fail; - r->text_buf = data_alloc(rb, &rb->text_data_ring, r->text_buf_size, + r->text_buf = data_alloc(rb, r->text_buf_size, &d->text_blk_lpos, id); } else { if (!get_data(&rb->text_data_ring, &d->text_blk_lpos, &data_size)) @@ -1421,7 +1420,7 @@ bool prb_reserve_in_last(struct prb_reserved_entry *e, struct printk_ringbuffer if (r->text_buf_size > max_size) goto fail; - r->text_buf = data_realloc(rb, &rb->text_data_ring, r->text_buf_size, + r->text_buf = data_realloc(rb, r->text_buf_size, &d->text_blk_lpos, id); } if (r->text_buf_size && !r->text_buf) @@ -1549,8 +1548,7 @@ bool prb_reserve(struct prb_reserved_entry *e, struct printk_ringbuffer *rb, if (info->seq > 0) desc_make_final(desc_ring, DESC_ID(id - 1)); - r->text_buf = data_alloc(rb, &rb->text_data_ring, r->text_buf_size, - &d->text_blk_lpos, id); + r->text_buf = data_alloc(rb, r->text_buf_size, &d->text_blk_lpos, id); /* If text data allocation fails, a data-less record is committed. */ if (r->text_buf_size && !r->text_buf) { prb_commit(e); From 77433830ed164a0bc38dd43877bab3f7f7fd7fa3 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Sun, 15 Nov 2020 20:35:30 -0800 Subject: [PATCH 16/29] powerpc: boot: include compiler_attributes.h The kernel uses `-include` to include include/linux/compiler_types.h into all translation units (see scripts/Makefile.lib), which #includes compiler_attributes.h. arch/powerpc/boot/ uses different compiler flags from the rest of the kernel. As such, it doesn't contain the definitions from these headers, and redefines a few that it needs. For the purpose of enabling -Wimplicit-fallthrough for ppc, include compiler_attributes.h via `-include`. It was also noted in 6a9dc5fd6170 that we could -D__KERNEL__ and -include compiler_types.h like the main kernel does, though testing that produces a whole sea of warnings to cleanup. This approach is minimally invasive. And it also helps to entice a cleanup. Signed-off-by: Nick Desaulniers Tested-by: Nathan Chancellor Reviewed-by: Nathan Chancellor Acked-by: Gustavo A. R. Silva Acked-by: Miguel Ojeda Acked-by: Michael Ellerman Link: https://github.com/ClangBuiltLinux/linux/issues/236 [ Gustavo: Massage a bit as per Miguel's suggestion. ] Signed-off-by: Gustavo A. R. Silva --- arch/powerpc/boot/Makefile | 1 + arch/powerpc/boot/decompress.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index f8ce6d2dde7b..1659963a8f1d 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -31,6 +31,7 @@ endif BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \ -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \ + -include $(srctree)/include/linux/compiler_attributes.h \ $(LINUXINCLUDE) ifdef CONFIG_PPC64_BOOT_WRAPPER diff --git a/arch/powerpc/boot/decompress.c b/arch/powerpc/boot/decompress.c index 8bf39ef7d2df..6098b879ac97 100644 --- a/arch/powerpc/boot/decompress.c +++ b/arch/powerpc/boot/decompress.c @@ -21,7 +21,6 @@ #define STATIC static #define INIT -#define __always_inline inline /* * The build process will copy the required zlib source files and headers From 4c1ca831adb1010e473a18eb01b3fbef7595f230 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Sun, 15 Nov 2020 20:35:31 -0800 Subject: [PATCH 17/29] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/" This reverts commit 6a9dc5fd6170 ("lib: Revert use of fallthrough pseudo-keyword in lib/") Now that we can build arch/powerpc/boot/ free of -Wimplicit-fallthrough, re-enable these fixes for lib/. Signed-off-by: Nick Desaulniers Tested-by: Nathan Chancellor Reviewed-by: Nathan Chancellor Reviewed-by: Gustavo A. R. Silva Reviewed-by: Miguel Ojeda Link: https://github.com/ClangBuiltLinux/linux/issues/236 Signed-off-by: Gustavo A. R. Silva --- lib/asn1_decoder.c | 4 ++-- lib/assoc_array.c | 2 +- lib/bootconfig.c | 4 ++-- lib/cmdline.c | 10 +++++----- lib/dim/net_dim.c | 2 +- lib/dim/rdma_dim.c | 4 ++-- lib/glob.c | 2 +- lib/siphash.c | 36 ++++++++++++++++++------------------ lib/ts_fsm.c | 2 +- lib/vsprintf.c | 14 +++++++------- lib/xz/xz_dec_lzma2.c | 4 ++-- lib/xz/xz_dec_stream.c | 16 ++++++++-------- lib/zstd/bitstream.h | 10 +++++----- lib/zstd/compress.c | 2 +- lib/zstd/decompress.c | 12 ++++++------ lib/zstd/huf_compress.c | 4 ++-- 16 files changed, 64 insertions(+), 64 deletions(-) diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c index 58f72b25f8e9..13da529e2e72 100644 --- a/lib/asn1_decoder.c +++ b/lib/asn1_decoder.c @@ -381,7 +381,7 @@ next_op: case ASN1_OP_END_SET_ACT: if (unlikely(!(flags & FLAG_MATCHED))) goto tag_mismatch; - /* fall through */ + fallthrough; case ASN1_OP_END_SEQ: case ASN1_OP_END_SET_OF: @@ -448,7 +448,7 @@ next_op: pc += asn1_op_lengths[op]; goto next_op; } - /* fall through */ + fallthrough; case ASN1_OP_ACT: ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len); diff --git a/lib/assoc_array.c b/lib/assoc_array.c index 6f4bcf524554..04c98799c3ba 100644 --- a/lib/assoc_array.c +++ b/lib/assoc_array.c @@ -1113,7 +1113,7 @@ struct assoc_array_edit *assoc_array_delete(struct assoc_array *array, index_key)) goto found_leaf; } - /* fall through */ + fallthrough; case assoc_array_walk_tree_empty: case assoc_array_walk_found_wrong_shortcut: default: diff --git a/lib/bootconfig.c b/lib/bootconfig.c index 649ed44f199c..9f8c70a98fcf 100644 --- a/lib/bootconfig.c +++ b/lib/bootconfig.c @@ -827,7 +827,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos) q - 2); break; } - /* fall through */ + fallthrough; case '=': ret = xbc_parse_kv(&p, q, c); break; @@ -836,7 +836,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos) break; case '#': q = skip_comment(q); - /* fall through */ + fallthrough; case ';': case '\n': ret = xbc_parse_key(&p, q); diff --git a/lib/cmdline.c b/lib/cmdline.c index fbb9981a04a4..55768b4f3f58 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -132,23 +132,23 @@ unsigned long long memparse(const char *ptr, char **retptr) case 'E': case 'e': ret <<= 10; - /* fall through */ + fallthrough; case 'P': case 'p': ret <<= 10; - /* fall through */ + fallthrough; case 'T': case 't': ret <<= 10; - /* fall through */ + fallthrough; case 'G': case 'g': ret <<= 10; - /* fall through */ + fallthrough; case 'M': case 'm': ret <<= 10; - /* fall through */ + fallthrough; case 'K': case 'k': ret <<= 10; diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c index a4db51c21266..06811d866775 100644 --- a/lib/dim/net_dim.c +++ b/lib/dim/net_dim.c @@ -233,7 +233,7 @@ void net_dim(struct dim *dim, struct dim_sample end_sample) schedule_work(&dim->work); break; } - /* fall through */ + fallthrough; case DIM_START_MEASURE: dim_update_sample(end_sample.event_ctr, end_sample.pkt_ctr, end_sample.byte_ctr, &dim->start_sample); diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c index f7e26c7b4749..15462d54758d 100644 --- a/lib/dim/rdma_dim.c +++ b/lib/dim/rdma_dim.c @@ -59,7 +59,7 @@ static bool rdma_dim_decision(struct dim_stats *curr_stats, struct dim *dim) break; case DIM_STATS_WORSE: dim_turn(dim); - /* fall through */ + fallthrough; case DIM_STATS_BETTER: step_res = rdma_dim_step(dim); if (step_res == DIM_ON_EDGE) @@ -94,7 +94,7 @@ void rdma_dim(struct dim *dim, u64 completions) schedule_work(&dim->work); break; } - /* fall through */ + fallthrough; case DIM_START_MEASURE: dim->state = DIM_MEASURE_IN_PROGRESS; dim_update_sample_with_comps(curr_sample->event_ctr, 0, 0, diff --git a/lib/glob.c b/lib/glob.c index 52e3ed7e4a9b..85ecbda45cd8 100644 --- a/lib/glob.c +++ b/lib/glob.c @@ -102,7 +102,7 @@ bool __pure glob_match(char const *pat, char const *str) break; case '\\': d = *pat++; - /* fall through */ + fallthrough; default: /* Literal character */ literal: if (c == d) { diff --git a/lib/siphash.c b/lib/siphash.c index c47bb6ff2149..a90112ee72a1 100644 --- a/lib/siphash.c +++ b/lib/siphash.c @@ -68,11 +68,11 @@ u64 __siphash_aligned(const void *data, size_t len, const siphash_key_t *key) bytemask_from_count(left))); #else switch (left) { - case 7: b |= ((u64)end[6]) << 48; /* fall through */ - case 6: b |= ((u64)end[5]) << 40; /* fall through */ - case 5: b |= ((u64)end[4]) << 32; /* fall through */ + case 7: b |= ((u64)end[6]) << 48; fallthrough; + case 6: b |= ((u64)end[5]) << 40; fallthrough; + case 5: b |= ((u64)end[4]) << 32; fallthrough; case 4: b |= le32_to_cpup(data); break; - case 3: b |= ((u64)end[2]) << 16; /* fall through */ + case 3: b |= ((u64)end[2]) << 16; fallthrough; case 2: b |= le16_to_cpup(data); break; case 1: b |= end[0]; } @@ -101,11 +101,11 @@ u64 __siphash_unaligned(const void *data, size_t len, const siphash_key_t *key) bytemask_from_count(left))); #else switch (left) { - case 7: b |= ((u64)end[6]) << 48; /* fall through */ - case 6: b |= ((u64)end[5]) << 40; /* fall through */ - case 5: b |= ((u64)end[4]) << 32; /* fall through */ + case 7: b |= ((u64)end[6]) << 48; fallthrough; + case 6: b |= ((u64)end[5]) << 40; fallthrough; + case 5: b |= ((u64)end[4]) << 32; fallthrough; case 4: b |= get_unaligned_le32(end); break; - case 3: b |= ((u64)end[2]) << 16; /* fall through */ + case 3: b |= ((u64)end[2]) << 16; fallthrough; case 2: b |= get_unaligned_le16(end); break; case 1: b |= end[0]; } @@ -268,11 +268,11 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key) bytemask_from_count(left))); #else switch (left) { - case 7: b |= ((u64)end[6]) << 48; /* fall through */ - case 6: b |= ((u64)end[5]) << 40; /* fall through */ - case 5: b |= ((u64)end[4]) << 32; /* fall through */ + case 7: b |= ((u64)end[6]) << 48; fallthrough; + case 6: b |= ((u64)end[5]) << 40; fallthrough; + case 5: b |= ((u64)end[4]) << 32; fallthrough; case 4: b |= le32_to_cpup(data); break; - case 3: b |= ((u64)end[2]) << 16; /* fall through */ + case 3: b |= ((u64)end[2]) << 16; fallthrough; case 2: b |= le16_to_cpup(data); break; case 1: b |= end[0]; } @@ -301,11 +301,11 @@ u32 __hsiphash_unaligned(const void *data, size_t len, bytemask_from_count(left))); #else switch (left) { - case 7: b |= ((u64)end[6]) << 48; /* fall through */ - case 6: b |= ((u64)end[5]) << 40; /* fall through */ - case 5: b |= ((u64)end[4]) << 32; /* fall through */ + case 7: b |= ((u64)end[6]) << 48; fallthrough; + case 6: b |= ((u64)end[5]) << 40; fallthrough; + case 5: b |= ((u64)end[4]) << 32; fallthrough; case 4: b |= get_unaligned_le32(end); break; - case 3: b |= ((u64)end[2]) << 16; /* fall through */ + case 3: b |= ((u64)end[2]) << 16; fallthrough; case 2: b |= get_unaligned_le16(end); break; case 1: b |= end[0]; } @@ -431,7 +431,7 @@ u32 __hsiphash_aligned(const void *data, size_t len, const hsiphash_key_t *key) v0 ^= m; } switch (left) { - case 3: b |= ((u32)end[2]) << 16; /* fall through */ + case 3: b |= ((u32)end[2]) << 16; fallthrough; case 2: b |= le16_to_cpup(data); break; case 1: b |= end[0]; } @@ -454,7 +454,7 @@ u32 __hsiphash_unaligned(const void *data, size_t len, v0 ^= m; } switch (left) { - case 3: b |= ((u32)end[2]) << 16; /* fall through */ + case 3: b |= ((u32)end[2]) << 16; fallthrough; case 2: b |= get_unaligned_le16(end); break; case 1: b |= end[0]; } diff --git a/lib/ts_fsm.c b/lib/ts_fsm.c index ab749ec10ab5..64fd9015ad80 100644 --- a/lib/ts_fsm.c +++ b/lib/ts_fsm.c @@ -193,7 +193,7 @@ startover: TOKEN_MISMATCH(); block_idx++; - /* fall through */ + fallthrough; case TS_FSM_ANY: if (next == NULL) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 14c9a6af1b23..d3c5c16f391c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1265,7 +1265,7 @@ char *mac_address_string(char *buf, char *end, u8 *addr, case 'R': reversed = true; - /* fall through */ + fallthrough; default: separator = ':'; @@ -1682,7 +1682,7 @@ char *uuid_string(char *buf, char *end, const u8 *addr, switch (*(++fmt)) { case 'L': uc = true; - /* fall through */ + fallthrough; case 'l': index = guid_index; break; @@ -2219,7 +2219,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'S': case 's': ptr = dereference_symbol_descriptor(ptr); - /* fall through */ + fallthrough; case 'B': return symbol_string(buf, end, ptr, spec, fmt); case 'R': @@ -2450,7 +2450,7 @@ qualifier: case 'x': spec->flags |= SMALL; - /* fall through */ + fallthrough; case 'X': spec->base = 16; @@ -2468,7 +2468,7 @@ qualifier: * utility, treat it as any other invalid or * unsupported format specifier. */ - /* fall through */ + fallthrough; default: WARN_ONCE(1, "Please remove unsupported %%%c in format string\n", *fmt); @@ -3411,10 +3411,10 @@ int vsscanf(const char *buf, const char *fmt, va_list args) break; case 'i': base = 0; - /* fall through */ + fallthrough; case 'd': is_sign = true; - /* fall through */ + fallthrough; case 'u': break; case '%': diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c index 65a1aad8c223..ca2603abee08 100644 --- a/lib/xz/xz_dec_lzma2.c +++ b/lib/xz/xz_dec_lzma2.c @@ -1043,7 +1043,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, s->lzma2.sequence = SEQ_LZMA_PREPARE; - /* fall through */ + fallthrough; case SEQ_LZMA_PREPARE: if (s->lzma2.compressed < RC_INIT_BYTES) @@ -1055,7 +1055,7 @@ XZ_EXTERN enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, s->lzma2.compressed -= RC_INIT_BYTES; s->lzma2.sequence = SEQ_LZMA_RUN; - /* fall through */ + fallthrough; case SEQ_LZMA_RUN: /* diff --git a/lib/xz/xz_dec_stream.c b/lib/xz/xz_dec_stream.c index 32ab2a08b7cb..fea86deaaa01 100644 --- a/lib/xz/xz_dec_stream.c +++ b/lib/xz/xz_dec_stream.c @@ -583,7 +583,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) if (ret != XZ_OK) return ret; - /* fall through */ + fallthrough; case SEQ_BLOCK_START: /* We need one byte of input to continue. */ @@ -608,7 +608,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->temp.pos = 0; s->sequence = SEQ_BLOCK_HEADER; - /* fall through */ + fallthrough; case SEQ_BLOCK_HEADER: if (!fill_temp(s, b)) @@ -620,7 +620,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->sequence = SEQ_BLOCK_UNCOMPRESS; - /* fall through */ + fallthrough; case SEQ_BLOCK_UNCOMPRESS: ret = dec_block(s, b); @@ -629,7 +629,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->sequence = SEQ_BLOCK_PADDING; - /* fall through */ + fallthrough; case SEQ_BLOCK_PADDING: /* @@ -651,7 +651,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->sequence = SEQ_BLOCK_CHECK; - /* fall through */ + fallthrough; case SEQ_BLOCK_CHECK: if (s->check_type == XZ_CHECK_CRC32) { @@ -675,7 +675,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->sequence = SEQ_INDEX_PADDING; - /* fall through */ + fallthrough; case SEQ_INDEX_PADDING: while ((s->index.size + (b->in_pos - s->in_start)) @@ -699,7 +699,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->sequence = SEQ_INDEX_CRC32; - /* fall through */ + fallthrough; case SEQ_INDEX_CRC32: ret = crc32_validate(s, b); @@ -709,7 +709,7 @@ static enum xz_ret dec_main(struct xz_dec *s, struct xz_buf *b) s->temp.size = STREAM_HEADER_SIZE; s->sequence = SEQ_STREAM_FOOTER; - /* fall through */ + fallthrough; case SEQ_STREAM_FOOTER: if (!fill_temp(s, b)) diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h index 3a49784d5c61..7c65c66e41fd 100644 --- a/lib/zstd/bitstream.h +++ b/lib/zstd/bitstream.h @@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s bitD->bitContainer = *(const BYTE *)(bitD->start); switch (srcSize) { case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16); - /* fall through */ + fallthrough; case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24); - /* fall through */ + fallthrough; case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32); - /* fall through */ + fallthrough; case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24; - /* fall through */ + fallthrough; case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16; - /* fall through */ + fallthrough; case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8; default:; } diff --git a/lib/zstd/compress.c b/lib/zstd/compress.c index 5e0b67003e55..b080264ed3ad 100644 --- a/lib/zstd/compress.c +++ b/lib/zstd/compress.c @@ -3182,7 +3182,7 @@ static size_t ZSTD_compressStream_generic(ZSTD_CStream *zcs, void *dst, size_t * zcs->outBuffFlushedSize = 0; zcs->stage = zcss_flush; /* pass-through to flush stage */ } - /* fall through */ + fallthrough; case zcss_flush: { size_t const toFlush = zcs->outBuffContentSize - zcs->outBuffFlushedSize; diff --git a/lib/zstd/decompress.c b/lib/zstd/decompress.c index db6761ea4deb..66cd487a326a 100644 --- a/lib/zstd/decompress.c +++ b/lib/zstd/decompress.c @@ -442,7 +442,7 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx *dctx, const void *src, size_t srcSize case set_repeat: if (dctx->litEntropy == 0) return ERROR(dictionary_corrupted); - /* fall through */ + fallthrough; case set_compressed: if (srcSize < 5) return ERROR(corruption_detected); /* srcSize >= MIN_CBLOCK_SIZE == 3; here we need up to 5 for case 3 */ @@ -1768,7 +1768,7 @@ size_t ZSTD_decompressContinue(ZSTD_DCtx *dctx, void *dst, size_t dstCapacity, c return 0; } dctx->expected = 0; /* not necessary to copy more */ - /* fall through */ + fallthrough; case ZSTDds_decodeFrameHeader: memcpy(dctx->headerBuffer + ZSTD_frameHeaderSize_prefix, src, dctx->expected); @@ -2309,7 +2309,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB switch (zds->stage) { case zdss_init: ZSTD_resetDStream(zds); /* transparent reset on starting decoding a new frame */ - /* fall through */ + fallthrough; case zdss_loadHeader: { size_t const hSize = ZSTD_getFrameParams(&zds->fParams, zds->headerBuffer, zds->lhSize); @@ -2376,7 +2376,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB } zds->stage = zdss_read; } - /* fall through */ + fallthrough; case zdss_read: { size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx); @@ -2405,7 +2405,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB zds->stage = zdss_load; /* pass-through */ } - /* fall through */ + fallthrough; case zdss_load: { size_t const neededInSize = ZSTD_nextSrcSizeToDecompress(zds->dctx); @@ -2438,7 +2438,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream *zds, ZSTD_outBuffer *output, ZSTD_inB /* pass-through */ } } - /* fall through */ + fallthrough; case zdss_flush: { size_t const toFlushSize = zds->outEnd - zds->outStart; diff --git a/lib/zstd/huf_compress.c b/lib/zstd/huf_compress.c index e727812d12aa..08b4ae80aed4 100644 --- a/lib/zstd/huf_compress.c +++ b/lib/zstd/huf_compress.c @@ -556,9 +556,9 @@ size_t HUF_compress1X_usingCTable(void *dst, size_t dstSize, const void *src, si n = srcSize & ~3; /* join to mod 4 */ switch (srcSize & 3) { case 3: HUF_encodeSymbol(&bitC, ip[n + 2], CTable); HUF_FLUSHBITS_2(&bitC); - /* fall through */ + fallthrough; case 2: HUF_encodeSymbol(&bitC, ip[n + 1], CTable); HUF_FLUSHBITS_1(&bitC); - /* fall through */ + fallthrough; case 1: HUF_encodeSymbol(&bitC, ip[n + 0], CTable); HUF_FLUSHBITS(&bitC); case 0: default:; From 49a41365052849be798716b374fabd436cce3ad0 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Sun, 15 Nov 2020 20:35:32 -0800 Subject: [PATCH 18/29] powerpc: fix -Wimplicit-fallthrough The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. Clang will still warn on cases where there is a fallthrough to an immediate break. Add explicit breaks for those cases. Signed-off-by: Nick Desaulniers Tested-by: Nathan Chancellor Reviewed-by: Gustavo A. R. Silva Reviewed-by: Miguel Ojeda Reviewed-by: Nathan Chancellor Acked-by: Michael Ellerman Link: https://github.com/ClangBuiltLinux/linux/issues/236 Signed-off-by: Gustavo A. R. Silva --- arch/powerpc/kernel/prom_init.c | 1 + arch/powerpc/kernel/uprobes.c | 1 + arch/powerpc/perf/imc-pmu.c | 1 + 3 files changed, 3 insertions(+) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 38ae5933d917..e9d4eb6144e1 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -355,6 +355,7 @@ static int __init prom_strtobool(const char *s, bool *res) default: break; } + break; default: break; } diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c index d200e7df7167..e8a63713e655 100644 --- a/arch/powerpc/kernel/uprobes.c +++ b/arch/powerpc/kernel/uprobes.c @@ -141,6 +141,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self, case DIE_SSTEP: if (uprobe_post_sstep_notifier(regs)) return NOTIFY_STOP; + break; default: break; } diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c index 7b25548ec42b..e106909ff9c3 100644 --- a/arch/powerpc/perf/imc-pmu.c +++ b/arch/powerpc/perf/imc-pmu.c @@ -1500,6 +1500,7 @@ static int update_pmu_ops(struct imc_pmu *pmu) pmu->pmu.stop = trace_imc_event_stop; pmu->pmu.read = trace_imc_event_read; pmu->attr_groups[IMC_FORMAT_ATTR] = &trace_imc_format_group; + break; default: break; } From 36f9ff9e03de89691274a6aec45aa079bd3ae405 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 19 Nov 2020 07:11:44 -0600 Subject: [PATCH 19/29] lib: Fix fall-through warnings for Clang In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple warnings by explicitly adding multiple break statements instead of letting the code fall through to the next case, and by replacing a number of /* fall through */ comments with the new pseudo-keyword macro fallthrough. Notice that Clang doesn't recognize /* Fall through */ comments as implicit fall-through markings. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva --- lib/cmdline.c | 1 + lib/kstrtox.c | 1 + lib/nlattr.c | 2 +- lib/vsprintf.c | 1 + lib/zlib_inflate/inflate.c | 24 ++++++++++++------------ lib/zstd/bitstream.h | 1 + lib/zstd/huf_compress.c | 1 + 7 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 55768b4f3f58..d11fdc579170 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -153,6 +153,7 @@ unsigned long long memparse(const char *ptr, char **retptr) case 'k': ret <<= 10; endptr++; + fallthrough; default: break; } diff --git a/lib/kstrtox.c b/lib/kstrtox.c index a14ccf905055..a118b0b1e9b2 100644 --- a/lib/kstrtox.c +++ b/lib/kstrtox.c @@ -355,6 +355,7 @@ int kstrtobool(const char *s, bool *res) default: break; } + break; default: break; } diff --git a/lib/nlattr.c b/lib/nlattr.c index 74019c8ebf6b..19372355557a 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -432,7 +432,7 @@ static int validate_nla(const struct nlattr *nla, int maxtype, err = -EINVAL; goto out_err; } - /* fall through */ + fallthrough; case NLA_STRING: if (attrlen < 1) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index d3c5c16f391c..3b53c73580c5 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2459,6 +2459,7 @@ qualifier: case 'd': case 'i': spec->flags |= SIGN; + break; case 'u': break; diff --git a/lib/zlib_inflate/inflate.c b/lib/zlib_inflate/inflate.c index 67cc9b08ae9d..ee39b5eb71f7 100644 --- a/lib/zlib_inflate/inflate.c +++ b/lib/zlib_inflate/inflate.c @@ -396,7 +396,7 @@ int zlib_inflate(z_streamp strm, int flush) strm->adler = state->check = REVERSE(hold); INITBITS(); state->mode = DICT; - /* fall through */ + fallthrough; case DICT: if (state->havedict == 0) { RESTORE(); @@ -404,10 +404,10 @@ int zlib_inflate(z_streamp strm, int flush) } strm->adler = state->check = zlib_adler32(0L, NULL, 0); state->mode = TYPE; - /* fall through */ + fallthrough; case TYPE: if (flush == Z_BLOCK) goto inf_leave; - /* fall through */ + fallthrough; case TYPEDO: INFLATE_TYPEDO_HOOK(strm, flush); if (state->last) { @@ -446,7 +446,7 @@ int zlib_inflate(z_streamp strm, int flush) state->length = (unsigned)hold & 0xffff; INITBITS(); state->mode = COPY; - /* fall through */ + fallthrough; case COPY: copy = state->length; if (copy) { @@ -480,7 +480,7 @@ int zlib_inflate(z_streamp strm, int flush) #endif state->have = 0; state->mode = LENLENS; - /* fall through */ + fallthrough; case LENLENS: while (state->have < state->ncode) { NEEDBITS(3); @@ -501,7 +501,7 @@ int zlib_inflate(z_streamp strm, int flush) } state->have = 0; state->mode = CODELENS; - /* fall through */ + fallthrough; case CODELENS: while (state->have < state->nlen + state->ndist) { for (;;) { @@ -575,7 +575,7 @@ int zlib_inflate(z_streamp strm, int flush) break; } state->mode = LEN; - /* fall through */ + fallthrough; case LEN: if (have >= 6 && left >= 258) { RESTORE(); @@ -615,7 +615,7 @@ int zlib_inflate(z_streamp strm, int flush) } state->extra = (unsigned)(this.op) & 15; state->mode = LENEXT; - /* fall through */ + fallthrough; case LENEXT: if (state->extra) { NEEDBITS(state->extra); @@ -623,7 +623,7 @@ int zlib_inflate(z_streamp strm, int flush) DROPBITS(state->extra); } state->mode = DIST; - /* fall through */ + fallthrough; case DIST: for (;;) { this = state->distcode[BITS(state->distbits)]; @@ -649,7 +649,7 @@ int zlib_inflate(z_streamp strm, int flush) state->offset = (unsigned)this.val; state->extra = (unsigned)(this.op) & 15; state->mode = DISTEXT; - /* fall through */ + fallthrough; case DISTEXT: if (state->extra) { NEEDBITS(state->extra); @@ -669,7 +669,7 @@ int zlib_inflate(z_streamp strm, int flush) break; } state->mode = MATCH; - /* fall through */ + fallthrough; case MATCH: if (left == 0) goto inf_leave; copy = out - left; @@ -720,7 +720,7 @@ int zlib_inflate(z_streamp strm, int flush) INITBITS(); } state->mode = DONE; - /* fall through */ + fallthrough; case DONE: ret = Z_STREAM_END; goto inf_leave; diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h index 7c65c66e41fd..5d6343c1a909 100644 --- a/lib/zstd/bitstream.h +++ b/lib/zstd/bitstream.h @@ -269,6 +269,7 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16; fallthrough; case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8; + fallthrough; default:; } { diff --git a/lib/zstd/huf_compress.c b/lib/zstd/huf_compress.c index 08b4ae80aed4..fd32838c185f 100644 --- a/lib/zstd/huf_compress.c +++ b/lib/zstd/huf_compress.c @@ -560,6 +560,7 @@ size_t HUF_compress1X_usingCTable(void *dst, size_t dstSize, const void *src, si case 2: HUF_encodeSymbol(&bitC, ip[n + 1], CTable); HUF_FLUSHBITS_1(&bitC); fallthrough; case 1: HUF_encodeSymbol(&bitC, ip[n + 0], CTable); HUF_FLUSHBITS(&bitC); + fallthrough; case 0: default:; } From 757055ae8dedf5333af17b3b5b4b70ba9bc9da4e Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Wed, 11 Nov 2020 14:54:49 +0100 Subject: [PATCH 20/29] init/console: Use ttynull as a fallback when there is no console stdin, stdout, and stderr standard I/O stream are created for the init process. They are not available when there is no console registered for /dev/console. It might lead to a crash when the init process tries to use them, see the commit 48021f98130880dd742 ("printk: handle blank console arguments passed in."). Normally, ttySX and ttyX consoles are used as a fallback when no consoles are defined via the command line, device tree, or SPCR. But there will be no console registered when an invalid console name is configured or when the configured consoles do not exist on the system. Users even try to avoid the console intentionally, for example, by using console="" or console=null. It is used on production systems where the serial port or terminal are not visible to users. Pushing messages to these consoles would just unnecessary slowdown the system. Make sure that stdin, stdout, stderr, and /dev/console are always available by a fallback to the existing ttynull driver. It has been implemented for exactly this purpose but it was used only when explicitly configured. Reviewed-by: Greg Kroah-Hartman Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Acked-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20201111135450.11214-2-pmladek@suse.com --- drivers/tty/Kconfig | 14 -------------- drivers/tty/Makefile | 3 +-- drivers/tty/ttynull.c | 18 ++++++++++++++++++ include/linux/console.h | 3 +++ init/main.c | 10 ++++++++-- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig index 93fd984eb2f5..ca359bbd62f5 100644 --- a/drivers/tty/Kconfig +++ b/drivers/tty/Kconfig @@ -428,20 +428,6 @@ config MIPS_EJTAG_FDC_KGDB_CHAN help FDC channel number to use for KGDB. -config NULL_TTY - tristate "NULL TTY driver" - help - Say Y here if you want a NULL TTY which simply discards messages. - - This is useful to allow userspace applications which expect a console - device to work without modifications even when no console is - available or desired. - - In order to use this driver, you should redirect the console to this - TTY, or boot the kernel with console=ttynull. - - If unsure, say N. - config TRACE_ROUTER tristate "Trace data router for MIPI P1149.7 cJTAG standard" depends on TRACE_SINK diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 020b1cd9294f..f6b6bee0422d 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -2,7 +2,7 @@ obj-$(CONFIG_TTY) += tty_io.o n_tty.o tty_ioctl.o tty_ldisc.o \ tty_buffer.o tty_port.o tty_mutex.o \ tty_ldsem.o tty_baudrate.o tty_jobctrl.o \ - n_null.o + n_null.o ttynull.o obj-$(CONFIG_LEGACY_PTYS) += pty.o obj-$(CONFIG_UNIX98_PTYS) += pty.o obj-$(CONFIG_AUDIT) += tty_audit.o @@ -25,7 +25,6 @@ obj-$(CONFIG_ISI) += isicom.o obj-$(CONFIG_MOXA_INTELLIO) += moxa.o obj-$(CONFIG_MOXA_SMARTIO) += mxser.o obj-$(CONFIG_NOZOMI) += nozomi.o -obj-$(CONFIG_NULL_TTY) += ttynull.o obj-$(CONFIG_ROCKETPORT) += rocket.o obj-$(CONFIG_SYNCLINK_GT) += synclink_gt.o obj-$(CONFIG_SYNCLINKMP) += synclinkmp.o diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c index 17f05b7eb6d3..eced70ec54e1 100644 --- a/drivers/tty/ttynull.c +++ b/drivers/tty/ttynull.c @@ -2,6 +2,13 @@ /* * Copyright (C) 2019 Axis Communications AB * + * The console is useful for userspace applications which expect a console + * device to work without modifications even when no console is available + * or desired. + * + * In order to use this driver, you should redirect the console to this + * TTY, or boot the kernel with console=ttynull. + * * Based on ttyprintk.c: * Copyright (C) 2010 Samo Pogacnik */ @@ -59,6 +66,17 @@ static struct console ttynull_console = { .device = ttynull_device, }; +void __init register_ttynull_console(void) +{ + if (!ttynull_driver) + return; + + if (add_preferred_console(ttynull_console.name, 0, NULL)) + return; + + register_console(&ttynull_console); +} + static int __init ttynull_init(void) { struct tty_driver *driver; diff --git a/include/linux/console.h b/include/linux/console.h index 4b1e26c4cb42..9c662e41cde5 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -187,9 +187,12 @@ extern int braille_register_console(struct console *, int index, extern int braille_unregister_console(struct console *); #ifdef CONFIG_TTY extern void console_sysfs_notify(void); +extern void register_ttynull_console(void); #else static inline void console_sysfs_notify(void) { } +static inline void register_ttynull_console(void) +{ } #endif extern bool console_suspend_enabled; diff --git a/init/main.c b/init/main.c index 1af84337cb18..b3fcf446f99e 100644 --- a/init/main.c +++ b/init/main.c @@ -1468,8 +1468,14 @@ void __init console_on_rootfs(void) struct file *file = filp_open("/dev/console", O_RDWR, 0); if (IS_ERR(file)) { - pr_err("Warning: unable to open an initial console.\n"); - return; + pr_err("Warning: unable to open an initial console. Fallback to ttynull.\n"); + register_ttynull_console(); + + file = filp_open("/dev/console", O_RDWR, 0); + if (IS_ERR(file)) { + pr_err("Warning: Failed to add ttynull console. No stdin, stdout, and stderr for the init process!\n"); + return; + } } init_dup(file); init_dup(file); From 3cffa06aeef7ece30f6b5ac0ea51f264e8fea4d0 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Wed, 11 Nov 2020 14:54:50 +0100 Subject: [PATCH 21/29] printk/console: Allow to disable console output by using console="" or console=null The commit 48021f98130880dd74 ("printk: handle blank console arguments passed in.") prevented crash caused by empty console= parameter value. Unfortunately, this value is widely used on Chromebooks to disable the console output. The above commit caused performance regression because the messages were pushed on slow console even though nobody was watching it. Use ttynull driver explicitly for console="" and console=null parameters. It has been created for exactly this purpose. It causes that preferred_console is set. As a result, ttySX and ttyX are not used as a fallback. And only ttynull console gets registered by default. It still allows to register other consoles either by additional console= parameters or SPCR. It prevents regression because it worked this way even before. Also it is a sane semantic. Preventing output on all consoles should be done another way, for example, by introducing mute_console parameter. Link: https://lore.kernel.org/r/20201006025935.GA597@jagdpanzerIV.localdomain Suggested-by: Sergey Senozhatsky Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Acked-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20201111135450.11214-3-pmladek@suse.com --- kernel/printk/printk.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fe64a49344bf..ac440b879a2c 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2189,8 +2189,15 @@ static int __init console_setup(char *str) char *s, *options, *brl_options = NULL; int idx; - if (str[0] == 0) + /* + * console="" or console=null have been suggested as a way to + * disable console output. Use ttynull that has been created + * for exacly this purpose. + */ + if (str[0] == 0 || strcmp(str, "null") == 0) { + __add_preferred_console("ttynull", 0, NULL, NULL, true); return 1; + } if (_braille_console_setup(&str, &brl_options)) return 1; From f3ed003e64fe7faecbe4c34bd2a1f5571a23f05a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 26 Oct 2020 18:59:27 +0200 Subject: [PATCH 22/29] kunit: Introduce get_file_path() helper Helper allows to derive file names depending on --build_dir argument. Signed-off-by: Andy Shevchenko Reviewed-by: Brendan Higgins Tested-by: Brendan Higgins Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_kernel.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 2e3cc0fac726..57c1724b7e5d 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -23,6 +23,11 @@ DEFAULT_KUNITCONFIG_PATH = 'arch/um/configs/kunit_defconfig' BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config' OUTFILE_PATH = 'test.log' +def get_file_path(build_dir, default): + if build_dir: + default = os.path.join(build_dir, default) + return default + class ConfigError(Exception): """Represents an error trying to configure the Linux kernel.""" @@ -97,9 +102,7 @@ class LinuxSourceTreeOperations(object): def linux_bin(self, params, timeout, build_dir): """Runs the Linux UML binary. Must be named 'linux'.""" - linux_bin = './linux' - if build_dir: - linux_bin = os.path.join(build_dir, 'linux') + linux_bin = get_file_path(build_dir, 'linux') outfile = get_outfile_path(build_dir) with open(outfile, 'w') as output: process = subprocess.Popen([linux_bin] + params, @@ -108,22 +111,13 @@ class LinuxSourceTreeOperations(object): process.wait(timeout) def get_kconfig_path(build_dir): - kconfig_path = KCONFIG_PATH - if build_dir: - kconfig_path = os.path.join(build_dir, KCONFIG_PATH) - return kconfig_path + return get_file_path(build_dir, KCONFIG_PATH) def get_kunitconfig_path(build_dir): - kunitconfig_path = KUNITCONFIG_PATH - if build_dir: - kunitconfig_path = os.path.join(build_dir, KUNITCONFIG_PATH) - return kunitconfig_path + return get_file_path(build_dir, KUNITCONFIG_PATH) def get_outfile_path(build_dir): - outfile_path = OUTFILE_PATH - if build_dir: - outfile_path = os.path.join(build_dir, OUTFILE_PATH) - return outfile_path + return get_file_path(build_dir, OUTFILE_PATH) class LinuxSourceTree(object): """Represents a Linux kernel source tree with KUnit tests.""" From 1f0e943df68ab407f523d9f47d96a535686a2293 Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Mon, 23 Nov 2020 14:57:59 -0800 Subject: [PATCH 23/29] Documentation: kunit: provide guidance for testing many inputs usage.rst goes into a detailed section about faking out classes, but currently lacks wording about how one might idiomatically test a range of inputs. Add a new chapter for "Common Patterns" and group "Isolating behvaior" and this new section under there. Give an example of how one might test a hash function via macros/helper funcs and a table-driven test and very briefly discuss pros and cons. Also highlight the KUNIT_EXPECT_*_MSG() variants (that aren't mentioned elsewhere [1]) which are particularly useful in these situations. It is also criminally underused at the moment, only appearing in 2 tests (both written by people involved in KUnit). [1] not even on https://www.kernel.org/doc/html/latest/dev-tools/kunit/api/test.html Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- Documentation/dev-tools/kunit/usage.rst | 83 +++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 9c28c518e6a3..d9fdc14f0677 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -15,10 +15,10 @@ project, see :doc:`start`. Organization of this document ============================= -This document is organized into two main sections: Testing and Isolating -Behavior. The first covers what unit tests are and how to use KUnit to write -them. The second covers how to use KUnit to isolate code and make it possible -to unit test code that was otherwise un-unit-testable. +This document is organized into two main sections: Testing and Common Patterns. +The first covers what unit tests are and how to use KUnit to write them. The +second covers common testing patterns, e.g. how to isolate code and make it +possible to unit test code that was otherwise un-unit-testable. Testing ======= @@ -218,8 +218,11 @@ test was built in or not). For more information on these types of things see the :doc:`api/test`. +Common Patterns +=============== + Isolating Behavior -================== +------------------ The most important aspect of unit testing that other forms of testing do not provide is the ability to limit the amount of code under test to a single unit. @@ -233,7 +236,7 @@ implementer, and architecture-specific functions which have definitions selected at compile time. Classes -------- +~~~~~~~ Classes are not a construct that is built into the C programming language; however, it is an easily derived concept. Accordingly, pretty much every project @@ -451,6 +454,74 @@ We can now use it to test ``struct eeprom_buffer``: destroy_eeprom_buffer(ctx->eeprom_buffer); } +Testing against multiple inputs +------------------------------- + +Testing just a few inputs might not be enough to have confidence that the code +works correctly, e.g. for a hash function. + +In such cases, it can be helpful to have a helper macro or function, e.g. this +fictitious example for ``sha1sum(1)`` + +.. code-block:: c + + /* Note: the cast is to satisfy overly strict type-checking. */ + #define TEST_SHA1(in, want) \ + sha1sum(in, out); \ + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, want, "sha1sum(%s)", in); + + char out[40]; + TEST_SHA1("hello world", "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed"); + TEST_SHA1("hello world!", "430ce34d020724ed75a196dfc2ad67c77772d169"); + + +Note the use of ``KUNIT_EXPECT_STREQ_MSG`` to give more context when it fails +and make it easier to track down. (Yes, in this example, ``want`` is likely +going to be unique enough on its own). + +The ``_MSG`` variants are even more useful when the same expectation is called +multiple times (in a loop or helper function) and thus the line number isn't +enough to identify what failed, like below. + +In some cases, it can be helpful to write a *table-driven test* instead, e.g. + +.. code-block:: c + + int i; + char out[40]; + + struct sha1_test_case { + const char *str; + const char *sha1; + }; + + struct sha1_test_case cases[] = { + { + .str = "hello world", + .sha1 = "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed", + }, + { + .str = "hello world!", + .sha1 = "430ce34d020724ed75a196dfc2ad67c77772d169", + }, + }; + for (i = 0; i < ARRAY_SIZE(cases); ++i) { + sha1sum(cases[i].str, out); + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, cases[i].sha1, + "sha1sum(%s)", cases[i].str); + } + + +There's more boilerplate involved, but it can: + +* be more readable when there are multiple inputs/outputs thanks to field names, + + * E.g. see ``fs/ext4/inode-test.c`` for an example of both. +* reduce duplication if test cases can be shared across multiple tests. + + * E.g. if we wanted to also test ``sha256sum``, we could add a ``sha256`` + field and reuse ``cases``. + .. _kunit-on-non-uml: KUnit on non-UML architectures From 0c7a7e1a8ff3fb6d01467b43c62760e6bf0afab3 Mon Sep 17 00:00:00 2001 From: David Gow Date: Mon, 9 Nov 2020 23:29:36 -0800 Subject: [PATCH 24/29] kunit: kunit_tool: Correctly parse diagnostic messages Currently, kunit_tool expects all diagnostic lines in test results to contain ": " somewhere, as both the subtest header and the crash report do. Fix this to accept any line starting with (minus indent) "# " as being a valid diagnostic line. This matches what the TAP spec[1] and the draft KTAP spec[2] are expecting. [1]: http://testanything.org/tap-specification.html [2]: https://lore.kernel.org/linux-kselftest/CY4PR13MB1175B804E31E502221BC8163FD830@CY4PR13MB1175.namprd13.prod.outlook.com/T/ Signed-off-by: David Gow Acked-by: Marco Elver Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_parser.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index bbfe1b4e4c1c..6614ec4d0898 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -135,8 +135,8 @@ def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: else: return False -SUBTEST_DIAGNOSTIC = re.compile(r'^[\s]+# .*?: (.*)$') -DIAGNOSTIC_CRASH_MESSAGE = 'kunit test case crashed!' +SUBTEST_DIAGNOSTIC = re.compile(r'^[\s]+# (.*)$') +DIAGNOSTIC_CRASH_MESSAGE = re.compile(r'^[\s]+# .*?: kunit test case crashed!$') def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: save_non_diagnositic(lines, test_case) @@ -146,7 +146,8 @@ def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: match = SUBTEST_DIAGNOSTIC.match(line) if match: test_case.log.append(lines.pop(0)) - if match.group(1) == DIAGNOSTIC_CRASH_MESSAGE: + crash_match = DIAGNOSTIC_CRASH_MESSAGE.match(line) + if crash_match: test_case.status = TestStatus.TEST_CRASHED return True else: From fadb08e7c7501ed42949e646c6865ba4ec5dd948 Mon Sep 17 00:00:00 2001 From: Arpitha Raghunandan <98.arpi@gmail.com> Date: Mon, 16 Nov 2020 11:10:35 +0530 Subject: [PATCH 25/29] kunit: Support for Parameterized Testing Implementation of support for parameterized testing in KUnit. This approach requires the creation of a test case using the KUNIT_CASE_PARAM() macro that accepts a generator function as input. This generator function should return the next parameter given the previous parameter in parameterized tests. It also provides a macro to generate common-case generators based on arrays. Generators may also optionally provide a human-readable description of parameters, which is displayed where available. Note, currently the result of each parameter run is displayed in diagnostic lines, and only the overall test case output summarizes TAP-compliant success or failure of all parameter runs. In future, when supported by kunit-tool, these can be turned into subsubtest outputs. Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> Co-developed-by: Marco Elver Signed-off-by: Marco Elver Reviewed-by: David Gow Tested-by: David Gow Signed-off-by: Shuah Khan --- include/kunit/test.h | 51 ++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 59 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/include/kunit/test.h b/include/kunit/test.h index df60be7e22ca..49601c4b98b8 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -94,6 +94,9 @@ struct kunit; /* Size of log associated with test. */ #define KUNIT_LOG_SIZE 512 +/* Maximum size of parameter description string. */ +#define KUNIT_PARAM_DESC_SIZE 128 + /* * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a * sub-subtest. See the "Subtests" section in @@ -107,6 +110,7 @@ struct kunit; * * @run_case: the function representing the actual test case. * @name: the name of the test case. + * @generate_params: the generator function for parameterized tests. * * A test case is a function with the signature, * ``void (*)(struct kunit *)`` @@ -141,6 +145,7 @@ struct kunit; struct kunit_case { void (*run_case)(struct kunit *test); const char *name; + const void* (*generate_params)(const void *prev, char *desc); /* private: internal use only. */ bool success; @@ -163,6 +168,27 @@ static inline char *kunit_status_to_string(bool status) */ #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name } +/** + * KUNIT_CASE_PARAM - A helper for creation a parameterized &struct kunit_case + * + * @test_name: a reference to a test case function. + * @gen_params: a reference to a parameter generator function. + * + * The generator function:: + * + * const void* gen_params(const void *prev, char *desc) + * + * is used to lazily generate a series of arbitrarily typed values that fit into + * a void*. The argument @prev is the previously returned value, which should be + * used to derive the next value; @prev is set to NULL on the initial generator + * call. When no more values are available, the generator must return NULL. + * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE) + * describing the parameter. + */ +#define KUNIT_CASE_PARAM(test_name, gen_params) \ + { .run_case = test_name, .name = #test_name, \ + .generate_params = gen_params } + /** * struct kunit_suite - describes a related collection of &struct kunit_case * @@ -208,6 +234,10 @@ struct kunit { const char *name; /* Read only after initialization! */ char *log; /* Points at case log after initialization */ struct kunit_try_catch try_catch; + /* param_value is the current parameter value for a test case. */ + const void *param_value; + /* param_index stores the index of the parameter in parameterized tests. */ + int param_index; /* * success starts as true, and may only be set to false during a * test case; thus, it is safe to update this across multiple @@ -1742,4 +1772,25 @@ do { \ fmt, \ ##__VA_ARGS__) +/** + * KUNIT_ARRAY_PARAM() - Define test parameter generator from an array. + * @name: prefix for the test parameter generator function. + * @array: array of test parameters. + * @get_desc: function to convert param to description; NULL to use default + * + * Define function @name_gen_params which uses @array to generate parameters. + */ +#define KUNIT_ARRAY_PARAM(name, array, get_desc) \ + static const void *name##_gen_params(const void *prev, char *desc) \ + { \ + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \ + if (__next - (array) < ARRAY_SIZE((array))) { \ + void (*__get_desc)(typeof(__next), char *) = get_desc; \ + if (__get_desc) \ + __get_desc(__next, desc); \ + return __next; \ + } \ + return NULL; \ + } + #endif /* _KUNIT_TEST_H */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 750704abe89a..ec9494e914ef 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -325,39 +325,72 @@ static void kunit_catch_run_case(void *data) * occur in a test case and reports them as failures. */ static void kunit_run_case_catch_errors(struct kunit_suite *suite, - struct kunit_case *test_case) + struct kunit_case *test_case, + struct kunit *test) { struct kunit_try_catch_context context; struct kunit_try_catch *try_catch; - struct kunit test; - kunit_init_test(&test, test_case->name, test_case->log); - try_catch = &test.try_catch; + kunit_init_test(test, test_case->name, test_case->log); + try_catch = &test->try_catch; kunit_try_catch_init(try_catch, - &test, + test, kunit_try_run_case, kunit_catch_run_case); - context.test = &test; + context.test = test; context.suite = suite; context.test_case = test_case; kunit_try_catch_run(try_catch, &context); - test_case->success = test.success; - - kunit_print_ok_not_ok(&test, true, test_case->success, - kunit_test_case_num(suite, test_case), - test_case->name); + test_case->success = test->success; } int kunit_run_tests(struct kunit_suite *suite) { + char param_desc[KUNIT_PARAM_DESC_SIZE]; struct kunit_case *test_case; kunit_print_subtest_start(suite); - kunit_suite_for_each_test_case(suite, test_case) - kunit_run_case_catch_errors(suite, test_case); + kunit_suite_for_each_test_case(suite, test_case) { + struct kunit test = { .param_value = NULL, .param_index = 0 }; + bool test_success = true; + + if (test_case->generate_params) { + /* Get initial param. */ + param_desc[0] = '\0'; + test.param_value = test_case->generate_params(NULL, param_desc); + } + + do { + kunit_run_case_catch_errors(suite, test_case, &test); + test_success &= test_case->success; + + if (test_case->generate_params) { + if (param_desc[0] == '\0') { + snprintf(param_desc, sizeof(param_desc), + "param-%d", test.param_index); + } + + kunit_log(KERN_INFO, &test, + KUNIT_SUBTEST_INDENT + "# %s: %s %d - %s", + test_case->name, + kunit_status_to_string(test.success), + test.param_index + 1, param_desc); + + /* Get next param. */ + param_desc[0] = '\0'; + test.param_value = test_case->generate_params(test.param_value, param_desc); + test.param_index++; + } + } while (test.param_value); + + kunit_print_ok_not_ok(&test, true, test_success, + kunit_test_case_num(suite, test_case), + test_case->name); + } kunit_print_subtest_end(suite); From 5f6b99d0287de2c2d0b5e7abcb0092d553ad804a Mon Sep 17 00:00:00 2001 From: Arpitha Raghunandan <98.arpi@gmail.com> Date: Mon, 16 Nov 2020 11:11:50 +0530 Subject: [PATCH 26/29] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Modify fs/ext4/inode-test.c to use the parameterized testing feature of KUnit. Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> Signed-off-by: Marco Elver Reviewed-by: Marco Elver Tested-by: David Gow Reviewed-by: David Gow Reviewed-by: Iurii Zaikin Acked-by: Theodore Ts'o Signed-off-by: Shuah Khan --- fs/ext4/inode-test.c | 316 ++++++++++++++++++++++--------------------- 1 file changed, 162 insertions(+), 154 deletions(-) diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c index d62d802c9c12..7935ea6cf92c 100644 --- a/fs/ext4/inode-test.c +++ b/fs/ext4/inode-test.c @@ -80,6 +80,145 @@ struct timestamp_expectation { bool lower_bound; }; +static const struct timestamp_expectation test_data[] = { + { + .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE, + .msb_set = true, + .lower_bound = true, + .extra_bits = 0, + .expected = {.tv_sec = -0x80000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NEG_NO_EXTRA_BITS_CASE, + .msb_set = true, + .lower_bound = false, + .extra_bits = 0, + .expected = {.tv_sec = -1LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = LOWER_BOUND_NONNEG_NO_EXTRA_BITS_CASE, + .msb_set = false, + .lower_bound = true, + .extra_bits = 0, + .expected = {0LL, 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NONNEG_NO_EXTRA_BITS_CASE, + .msb_set = false, + .lower_bound = false, + .extra_bits = 0, + .expected = {.tv_sec = 0x7fffffffLL, .tv_nsec = 0L}, + }, + + { + .test_case_name = LOWER_BOUND_NEG_LO_1_CASE, + .msb_set = true, + .lower_bound = true, + .extra_bits = 1, + .expected = {.tv_sec = 0x80000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NEG_LO_1_CASE, + .msb_set = true, + .lower_bound = false, + .extra_bits = 1, + .expected = {.tv_sec = 0xffffffffLL, .tv_nsec = 0L}, + }, + + { + .test_case_name = LOWER_BOUND_NONNEG_LO_1_CASE, + .msb_set = false, + .lower_bound = true, + .extra_bits = 1, + .expected = {.tv_sec = 0x100000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NONNEG_LO_1_CASE, + .msb_set = false, + .lower_bound = false, + .extra_bits = 1, + .expected = {.tv_sec = 0x17fffffffLL, .tv_nsec = 0L}, + }, + + { + .test_case_name = LOWER_BOUND_NEG_HI_1_CASE, + .msb_set = true, + .lower_bound = true, + .extra_bits = 2, + .expected = {.tv_sec = 0x180000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NEG_HI_1_CASE, + .msb_set = true, + .lower_bound = false, + .extra_bits = 2, + .expected = {.tv_sec = 0x1ffffffffLL, .tv_nsec = 0L}, + }, + + { + .test_case_name = LOWER_BOUND_NONNEG_HI_1_CASE, + .msb_set = false, + .lower_bound = true, + .extra_bits = 2, + .expected = {.tv_sec = 0x200000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NONNEG_HI_1_CASE, + .msb_set = false, + .lower_bound = false, + .extra_bits = 2, + .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NONNEG_HI_1_NS_1_CASE, + .msb_set = false, + .lower_bound = false, + .extra_bits = 6, + .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 1L}, + }, + + { + .test_case_name = LOWER_BOUND_NONNEG_HI_1_NS_MAX_CASE, + .msb_set = false, + .lower_bound = true, + .extra_bits = 0xFFFFFFFF, + .expected = {.tv_sec = 0x300000000LL, + .tv_nsec = MAX_NANOSECONDS}, + }, + + { + .test_case_name = LOWER_BOUND_NONNEG_EXTRA_BITS_1_CASE, + .msb_set = false, + .lower_bound = true, + .extra_bits = 3, + .expected = {.tv_sec = 0x300000000LL, .tv_nsec = 0L}, + }, + + { + .test_case_name = UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE, + .msb_set = false, + .lower_bound = false, + .extra_bits = 3, + .expected = {.tv_sec = 0x37fffffffLL, .tv_nsec = 0L}, + } +}; + +static void timestamp_expectation_to_desc(const struct timestamp_expectation *t, + char *desc) +{ + strscpy(desc, t->test_case_name, KUNIT_PARAM_DESC_SIZE); +} + +KUNIT_ARRAY_PARAM(ext4_inode, test_data, timestamp_expectation_to_desc); + static time64_t get_32bit_time(const struct timestamp_expectation * const test) { if (test->msb_set) { @@ -101,166 +240,35 @@ static time64_t get_32bit_time(const struct timestamp_expectation * const test) */ static void inode_test_xtimestamp_decoding(struct kunit *test) { - const struct timestamp_expectation test_data[] = { - { - .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE, - .msb_set = true, - .lower_bound = true, - .extra_bits = 0, - .expected = {.tv_sec = -0x80000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NEG_NO_EXTRA_BITS_CASE, - .msb_set = true, - .lower_bound = false, - .extra_bits = 0, - .expected = {.tv_sec = -1LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = LOWER_BOUND_NONNEG_NO_EXTRA_BITS_CASE, - .msb_set = false, - .lower_bound = true, - .extra_bits = 0, - .expected = {0LL, 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NONNEG_NO_EXTRA_BITS_CASE, - .msb_set = false, - .lower_bound = false, - .extra_bits = 0, - .expected = {.tv_sec = 0x7fffffffLL, .tv_nsec = 0L}, - }, - - { - .test_case_name = LOWER_BOUND_NEG_LO_1_CASE, - .msb_set = true, - .lower_bound = true, - .extra_bits = 1, - .expected = {.tv_sec = 0x80000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NEG_LO_1_CASE, - .msb_set = true, - .lower_bound = false, - .extra_bits = 1, - .expected = {.tv_sec = 0xffffffffLL, .tv_nsec = 0L}, - }, - - { - .test_case_name = LOWER_BOUND_NONNEG_LO_1_CASE, - .msb_set = false, - .lower_bound = true, - .extra_bits = 1, - .expected = {.tv_sec = 0x100000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NONNEG_LO_1_CASE, - .msb_set = false, - .lower_bound = false, - .extra_bits = 1, - .expected = {.tv_sec = 0x17fffffffLL, .tv_nsec = 0L}, - }, - - { - .test_case_name = LOWER_BOUND_NEG_HI_1_CASE, - .msb_set = true, - .lower_bound = true, - .extra_bits = 2, - .expected = {.tv_sec = 0x180000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NEG_HI_1_CASE, - .msb_set = true, - .lower_bound = false, - .extra_bits = 2, - .expected = {.tv_sec = 0x1ffffffffLL, .tv_nsec = 0L}, - }, - - { - .test_case_name = LOWER_BOUND_NONNEG_HI_1_CASE, - .msb_set = false, - .lower_bound = true, - .extra_bits = 2, - .expected = {.tv_sec = 0x200000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NONNEG_HI_1_CASE, - .msb_set = false, - .lower_bound = false, - .extra_bits = 2, - .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NONNEG_HI_1_NS_1_CASE, - .msb_set = false, - .lower_bound = false, - .extra_bits = 6, - .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 1L}, - }, - - { - .test_case_name = LOWER_BOUND_NONNEG_HI_1_NS_MAX_CASE, - .msb_set = false, - .lower_bound = true, - .extra_bits = 0xFFFFFFFF, - .expected = {.tv_sec = 0x300000000LL, - .tv_nsec = MAX_NANOSECONDS}, - }, - - { - .test_case_name = LOWER_BOUND_NONNEG_EXTRA_BITS_1_CASE, - .msb_set = false, - .lower_bound = true, - .extra_bits = 3, - .expected = {.tv_sec = 0x300000000LL, .tv_nsec = 0L}, - }, - - { - .test_case_name = UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE, - .msb_set = false, - .lower_bound = false, - .extra_bits = 3, - .expected = {.tv_sec = 0x37fffffffLL, .tv_nsec = 0L}, - } - }; - struct timespec64 timestamp; - int i; - for (i = 0; i < ARRAY_SIZE(test_data); ++i) { - timestamp.tv_sec = get_32bit_time(&test_data[i]); - ext4_decode_extra_time(×tamp, - cpu_to_le32(test_data[i].extra_bits)); + struct timestamp_expectation *test_param = + (struct timestamp_expectation *)(test->param_value); - KUNIT_EXPECT_EQ_MSG(test, - test_data[i].expected.tv_sec, - timestamp.tv_sec, - CASE_NAME_FORMAT, - test_data[i].test_case_name, - test_data[i].msb_set, - test_data[i].lower_bound, - test_data[i].extra_bits); - KUNIT_EXPECT_EQ_MSG(test, - test_data[i].expected.tv_nsec, - timestamp.tv_nsec, - CASE_NAME_FORMAT, - test_data[i].test_case_name, - test_data[i].msb_set, - test_data[i].lower_bound, - test_data[i].extra_bits); - } + timestamp.tv_sec = get_32bit_time(test_param); + ext4_decode_extra_time(×tamp, + cpu_to_le32(test_param->extra_bits)); + + KUNIT_EXPECT_EQ_MSG(test, + test_param->expected.tv_sec, + timestamp.tv_sec, + CASE_NAME_FORMAT, + test_param->test_case_name, + test_param->msb_set, + test_param->lower_bound, + test_param->extra_bits); + KUNIT_EXPECT_EQ_MSG(test, + test_param->expected.tv_nsec, + timestamp.tv_nsec, + CASE_NAME_FORMAT, + test_param->test_case_name, + test_param->msb_set, + test_param->lower_bound, + test_param->extra_bits); } static struct kunit_case ext4_inode_test_cases[] = { - KUNIT_CASE(inode_test_xtimestamp_decoding), + KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding, ext4_inode_gen_params), {} }; From 8d143c610b62f2820fbc97dc441d54ac326abe1a Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Mon, 30 Nov 2020 13:49:15 +0100 Subject: [PATCH 27/29] printk: remove obsolete dead assignment Commit 849f3127bb46 ("switch /dev/kmsg to ->write_iter()") refactored devkmsg_write() and left over a dead assignment on the variable 'len'. Hence, make clang-analyzer warns: kernel/printk/printk.c:744:4: warning: Value stored to 'len' is never read [clang-analyzer-deadcode.DeadStores] len -= endp - line; ^ Simply remove this obsolete dead assignment here. Link: https://lore.kernel.org/r/20201130124915.7573-1-lukas.bulwahn@gmail.com Signed-off-by: Lukas Bulwahn Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fe64a49344bf..6bffb01431dd 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -741,7 +741,6 @@ static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from) if (LOG_FACILITY(u) != 0) facility = LOG_FACILITY(u); endp++; - len -= endp - line; line = endp; } } From 6b916706f8f09348cfa4fdd3642ebf87d6a2a26b Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 9 Dec 2020 01:50:52 +0106 Subject: [PATCH 28/29] printk: inline log_output(),log_store() in vprintk_store() In preparation for removing logbuf_lock, inline log_output() and log_store() into vprintk_store(). This will simplify dealing with the various code branches and fallbacks that are possible. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20201209004453.17720-2-john.ogness@linutronix.de --- kernel/printk/printk.c | 145 +++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 78 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index bc1e3b5a97bd..150bfde41ba1 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -491,52 +491,6 @@ static void truncate_msg(u16 *text_len, u16 *trunc_msg_len) *trunc_msg_len = 0; } -/* insert record into the buffer, discard old ones, update heads */ -static int log_store(u32 caller_id, int facility, int level, - enum log_flags flags, u64 ts_nsec, - const struct dev_printk_info *dev_info, - const char *text, u16 text_len) -{ - struct prb_reserved_entry e; - struct printk_record r; - u16 trunc_msg_len = 0; - - prb_rec_init_wr(&r, text_len); - - if (!prb_reserve(&e, prb, &r)) { - /* truncate the message if it is too long for empty buffer */ - truncate_msg(&text_len, &trunc_msg_len); - prb_rec_init_wr(&r, text_len + trunc_msg_len); - /* survive when the log buffer is too small for trunc_msg */ - if (!prb_reserve(&e, prb, &r)) - return 0; - } - - /* fill message */ - memcpy(&r.text_buf[0], text, text_len); - if (trunc_msg_len) - memcpy(&r.text_buf[text_len], trunc_msg, trunc_msg_len); - r.info->text_len = text_len + trunc_msg_len; - r.info->facility = facility; - r.info->level = level & 7; - r.info->flags = flags & 0x1f; - if (ts_nsec > 0) - r.info->ts_nsec = ts_nsec; - else - r.info->ts_nsec = local_clock(); - r.info->caller_id = caller_id; - if (dev_info) - memcpy(&r.info->dev_info, dev_info, sizeof(r.info->dev_info)); - - /* A message without a trailing newline can be continued. */ - if (!(flags & LOG_NEWLINE)) - prb_commit(&e); - else - prb_final_commit(&e); - - return (text_len + trunc_msg_len); -} - int dmesg_restrict = IS_ENABLED(CONFIG_SECURITY_DMESG_RESTRICT); static int syslog_action_restricted(int type) @@ -1907,44 +1861,28 @@ static inline u32 printk_caller_id(void) 0x80000000 + raw_smp_processor_id(); } -static size_t log_output(int facility, int level, enum log_flags lflags, - const struct dev_printk_info *dev_info, - char *text, size_t text_len) -{ - const u32 caller_id = printk_caller_id(); - - if (lflags & LOG_CONT) { - struct prb_reserved_entry e; - struct printk_record r; - - prb_rec_init_wr(&r, text_len); - if (prb_reserve_in_last(&e, prb, &r, caller_id, LOG_LINE_MAX)) { - memcpy(&r.text_buf[r.info->text_len], text, text_len); - r.info->text_len += text_len; - if (lflags & LOG_NEWLINE) { - r.info->flags |= LOG_NEWLINE; - prb_final_commit(&e); - } else { - prb_commit(&e); - } - return text_len; - } - } - - /* Store it in the record log */ - return log_store(caller_id, facility, level, lflags, 0, - dev_info, text, text_len); -} - /* Must be called under logbuf_lock. */ int vprintk_store(int facility, int level, const struct dev_printk_info *dev_info, const char *fmt, va_list args) { + const u32 caller_id = printk_caller_id(); static char textbuf[LOG_LINE_MAX]; - char *text = textbuf; - size_t text_len; + struct prb_reserved_entry e; enum log_flags lflags = 0; + struct printk_record r; + u16 trunc_msg_len = 0; + char *text = textbuf; + u16 text_len; + u64 ts_nsec; + + /* + * Since the duration of printk() can vary depending on the message + * and state of the ringbuffer, grab the timestamp now so that it is + * close to the call of printk(). This provides a more deterministic + * timestamp with respect to the caller. + */ + ts_nsec = local_clock(); /* * The printf needs to come first; we need the syslog @@ -1983,7 +1921,58 @@ int vprintk_store(int facility, int level, if (dev_info) lflags |= LOG_NEWLINE; - return log_output(facility, level, lflags, dev_info, text, text_len); + if (lflags & LOG_CONT) { + prb_rec_init_wr(&r, text_len); + if (prb_reserve_in_last(&e, prb, &r, caller_id, LOG_LINE_MAX)) { + memcpy(&r.text_buf[r.info->text_len], text, text_len); + r.info->text_len += text_len; + + if (lflags & LOG_NEWLINE) { + r.info->flags |= LOG_NEWLINE; + prb_final_commit(&e); + } else { + prb_commit(&e); + } + + return text_len; + } + } + + /* + * Explicitly initialize the record before every prb_reserve() call. + * prb_reserve_in_last() and prb_reserve() purposely invalidate the + * structure when they fail. + */ + prb_rec_init_wr(&r, text_len); + if (!prb_reserve(&e, prb, &r)) { + /* truncate the message if it is too long for empty buffer */ + truncate_msg(&text_len, &trunc_msg_len); + + prb_rec_init_wr(&r, text_len + trunc_msg_len); + if (!prb_reserve(&e, prb, &r)) + return 0; + } + + /* fill message */ + memcpy(&r.text_buf[0], text, text_len); + if (trunc_msg_len) + memcpy(&r.text_buf[text_len], trunc_msg, trunc_msg_len); + r.info->text_len = text_len + trunc_msg_len; + r.info->facility = facility; + r.info->level = level & 7; + r.info->flags = lflags & 0x1f; + r.info->ts_nsec = ts_nsec; + r.info->caller_id = caller_id; + if (dev_info) + memcpy(&r.info->dev_info, dev_info, sizeof(r.info->dev_info)); + + /* A message without a trailing newline can be continued. */ + if (!(lflags & LOG_NEWLINE)) + prb_commit(&e); + else + prb_final_commit(&e); + + return (text_len + trunc_msg_len); } asmlinkage int vprintk_emit(int facility, int level, From b031a684bfd01d633c79d281bd0cf11c2f834ada Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 9 Dec 2020 01:50:53 +0106 Subject: [PATCH 29/29] printk: remove logbuf_lock writer-protection of ringbuffer Since the ringbuffer is lockless, there is no need for it to be protected by @logbuf_lock. Remove @logbuf_lock writer-protection of the ringbuffer. The reader-protection is not removed because some variables, used by readers, are using @logbuf_lock for synchronization: @syslog_seq, @syslog_time, @syslog_partial, @console_seq, struct kmsg_dumper. For PRINTK_NMI_DIRECT_CONTEXT_MASK, @logbuf_lock usage is not removed because it may be used for dumper synchronization. Without @logbuf_lock synchronization of vprintk_store() it is no longer possible to use the single static buffer for temporarily sprint'ing the message. Instead, use vsnprintf() to determine the length and perform the real vscnprintf() using the area reserved from the ringbuffer. This leads to suboptimal packing of the message data, but will result in less wasted storage than multiple per-cpu buffers to support lockless temporary sprint'ing. Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20201209004453.17720-3-john.ogness@linutronix.de --- kernel/printk/printk.c | 138 +++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 40 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 150bfde41ba1..c8847ee571f0 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1126,7 +1126,7 @@ void __init setup_log_buf(int early) new_descs, ilog2(new_descs_count), new_infos); - logbuf_lock_irqsave(flags); + printk_safe_enter_irqsave(flags); log_buf_len = new_log_buf_len; log_buf = new_log_buf; @@ -1143,7 +1143,7 @@ void __init setup_log_buf(int early) */ prb = &printk_rb_dynamic; - logbuf_unlock_irqrestore(flags); + printk_safe_exit_irqrestore(flags); if (seq != prb_next_seq(&printk_rb_static)) { pr_err("dropped %llu messages\n", @@ -1861,18 +1861,90 @@ static inline u32 printk_caller_id(void) 0x80000000 + raw_smp_processor_id(); } -/* Must be called under logbuf_lock. */ +/** + * parse_prefix - Parse level and control flags. + * + * @text: The terminated text message. + * @level: A pointer to the current level value, will be updated. + * @lflags: A pointer to the current log flags, will be updated. + * + * @level may be NULL if the caller is not interested in the parsed value. + * Otherwise the variable pointed to by @level must be set to + * LOGLEVEL_DEFAULT in order to be updated with the parsed value. + * + * @lflags may be NULL if the caller is not interested in the parsed value. + * Otherwise the variable pointed to by @lflags will be OR'd with the parsed + * value. + * + * Return: The length of the parsed level and control flags. + */ +static u16 parse_prefix(char *text, int *level, enum log_flags *lflags) +{ + u16 prefix_len = 0; + int kern_level; + + while (*text) { + kern_level = printk_get_level(text); + if (!kern_level) + break; + + switch (kern_level) { + case '0' ... '7': + if (level && *level == LOGLEVEL_DEFAULT) + *level = kern_level - '0'; + break; + case 'c': /* KERN_CONT */ + if (lflags) + *lflags |= LOG_CONT; + } + + prefix_len += 2; + text += 2; + } + + return prefix_len; +} + +static u16 printk_sprint(char *text, u16 size, int facility, enum log_flags *lflags, + const char *fmt, va_list args) +{ + u16 text_len; + + text_len = vscnprintf(text, size, fmt, args); + + /* Mark and strip a trailing newline. */ + if (text_len && text[text_len - 1] == '\n') { + text_len--; + *lflags |= LOG_NEWLINE; + } + + /* Strip log level and control flags. */ + if (facility == 0) { + u16 prefix_len; + + prefix_len = parse_prefix(text, NULL, NULL); + if (prefix_len) { + text_len -= prefix_len; + memmove(text, text + prefix_len, text_len); + } + } + + return text_len; +} + +__printf(4, 0) int vprintk_store(int facility, int level, const struct dev_printk_info *dev_info, const char *fmt, va_list args) { const u32 caller_id = printk_caller_id(); - static char textbuf[LOG_LINE_MAX]; struct prb_reserved_entry e; enum log_flags lflags = 0; struct printk_record r; u16 trunc_msg_len = 0; - char *text = textbuf; + char prefix_buf[8]; + u16 reserve_size; + va_list args2; u16 text_len; u64 ts_nsec; @@ -1885,35 +1957,21 @@ int vprintk_store(int facility, int level, ts_nsec = local_clock(); /* - * The printf needs to come first; we need the syslog - * prefix which might be passed-in as a parameter. + * The sprintf needs to come first since the syslog prefix might be + * passed in as a parameter. An extra byte must be reserved so that + * later the vscnprintf() into the reserved buffer has room for the + * terminating '\0', which is not counted by vsnprintf(). */ - text_len = vscnprintf(text, sizeof(textbuf), fmt, args); + va_copy(args2, args); + reserve_size = vsnprintf(&prefix_buf[0], sizeof(prefix_buf), fmt, args2) + 1; + va_end(args2); - /* mark and strip a trailing newline */ - if (text_len && text[text_len-1] == '\n') { - text_len--; - lflags |= LOG_NEWLINE; - } + if (reserve_size > LOG_LINE_MAX) + reserve_size = LOG_LINE_MAX; - /* strip kernel syslog prefix and extract log level or control flags */ - if (facility == 0) { - int kern_level; - - while ((kern_level = printk_get_level(text)) != 0) { - switch (kern_level) { - case '0' ... '7': - if (level == LOGLEVEL_DEFAULT) - level = kern_level - '0'; - break; - case 'c': /* KERN_CONT */ - lflags |= LOG_CONT; - } - - text_len -= 2; - text += 2; - } - } + /* Extract log level or control flags. */ + if (facility == 0) + parse_prefix(&prefix_buf[0], &level, &lflags); if (level == LOGLEVEL_DEFAULT) level = default_message_loglevel; @@ -1922,9 +1980,10 @@ int vprintk_store(int facility, int level, lflags |= LOG_NEWLINE; if (lflags & LOG_CONT) { - prb_rec_init_wr(&r, text_len); + prb_rec_init_wr(&r, reserve_size); if (prb_reserve_in_last(&e, prb, &r, caller_id, LOG_LINE_MAX)) { - memcpy(&r.text_buf[r.info->text_len], text, text_len); + text_len = printk_sprint(&r.text_buf[r.info->text_len], reserve_size, + facility, &lflags, fmt, args); r.info->text_len += text_len; if (lflags & LOG_NEWLINE) { @@ -1943,18 +2002,18 @@ int vprintk_store(int facility, int level, * prb_reserve_in_last() and prb_reserve() purposely invalidate the * structure when they fail. */ - prb_rec_init_wr(&r, text_len); + prb_rec_init_wr(&r, reserve_size); if (!prb_reserve(&e, prb, &r)) { /* truncate the message if it is too long for empty buffer */ - truncate_msg(&text_len, &trunc_msg_len); + truncate_msg(&reserve_size, &trunc_msg_len); - prb_rec_init_wr(&r, text_len + trunc_msg_len); + prb_rec_init_wr(&r, reserve_size + trunc_msg_len); if (!prb_reserve(&e, prb, &r)) return 0; } /* fill message */ - memcpy(&r.text_buf[0], text, text_len); + text_len = printk_sprint(&r.text_buf[0], reserve_size, facility, &lflags, fmt, args); if (trunc_msg_len) memcpy(&r.text_buf[text_len], trunc_msg, trunc_msg_len); r.info->text_len = text_len + trunc_msg_len; @@ -1995,10 +2054,9 @@ asmlinkage int vprintk_emit(int facility, int level, boot_delay_msec(level); printk_delay(); - /* This stops the holder of console_sem just where we want him */ - logbuf_lock_irqsave(flags); + printk_safe_enter_irqsave(flags); printed_len = vprintk_store(facility, level, dev_info, fmt, args); - logbuf_unlock_irqrestore(flags); + printk_safe_exit_irqrestore(flags); /* If called from the scheduler, we can not call up(). */ if (!in_sched) {