- Linux-kselftest-mirror - lists.linaro.org

[PATCH v8 00/14] prohibit pinning pages in ZONE_MOVABLE

by Pavel Tatashin

Changelog --------- v8 - Added reviewed by's from John Hubbard - Fixed subjects for selftests patches - Moved zero page check inside is_pinnable_page() as requested by Jason Gunthorpe. v7 - Added reviewed-by's - Fixed a compile bug on non-mmu builds reported by robot v6 Small update, but I wanted to send it out quicker, as it removes a controversial patch and replaces it with something sane. - Removed forcing FOLL_WRITE for longterm gup, instead added a patch to skip zero pages during migration. - Added reviewed-by's and minor log changes. v5 - Added the following patches to the beginning of series, which are fixes to the other existing problems with CMA migration code: mm/gup: check every subpage of a compound page during isolation mm/gup: return an error on migration failure mm/gup: check for isolation errors also at the beginning of series mm/gup: do not allow zero page for pinned pages - remove .gfp_mask/.reclaim_idx changes from mm/vmscan.c - update movable zone header comment in patch 8 instead of patch 3, fix the comment - Added acked, sign-offs - Updated commit logs based on feedback - Addressed issues reported by Michal and Jason. - Remove: #define PINNABLE_MIGRATE_MAX 10 #define PINNABLE_ISOLATE_MAX 100 Instead: fail on the first migration failure, and retry isolation forever as their failures are transient. - In self-set addressed some of the comments from John Hubbard, updated commit logs, and added comments. Renamed gup->flags with gup->test_flags. v4 - Address page migration comments. New patch: mm/gup: limit number of gup migration failures, honor failures Implements the limiting number of retries for migration failures, and also check for isolation failures. Added a test case into gup_test to verify that pages never long-term pinned in a movable zone, and also added tests to fault both in kernel and in userland. v3 - Merged with linux-next, which contains clean-up patch from Jason, therefore this series is reduced by two patches which did the same thing. v2 - Addressed all review comments - Added Reviewed-by's. - Renamed PF_MEMALLOC_NOMOVABLE to PF_MEMALLOC_PIN - Added is_pinnable_page() to check if page can be longterm pinned - Fixed gup fast path by checking is_in_pinnable_zone() - rename cma_page_list to movable_page_list - add a admin-guide note about handling pinned pages in ZONE_MOVABLE, updated caveat about pinned pages from linux/mmzone.h - Move current_gfp_context() to fast-path --------- When page is pinned it cannot be moved and its physical address stays the same until pages is unpinned. This is useful functionality to allows userland to implementation DMA access. For example, it is used by vfio in vfio_pin_pages(). However, this functionality breaks memory hotplug/hotremove assumptions that pages in ZONE_MOVABLE can always be migrated. This patch series fixes this issue by forcing new allocations during page pinning to omit ZONE_MOVABLE, and also to migrate any existing pages from ZONE_MOVABLE during pinning. It uses the same scheme logic that is currently used by CMA, and extends the functionality for all allocations. For more information read the discussion [1] about this problem. [1] https://lore.kernel.org/lkml/CA+CK2bBffHBxjmb9jmSKacm0fJMinyt3Nhk8Nx6iudcQS… Previous versions: v1 https://lore.kernel.org/lkml/20201202052330.474592-1-pasha.tatashin@soleen.… v2 https://lore.kernel.org/lkml/20201210004335.64634-1-pasha.tatashin@soleen.c… v3 https://lore.kernel.org/lkml/20201211202140.396852-1-pasha.tatashin@soleen.… v4 https://lore.kernel.org/lkml/20201217185243.3288048-1-pasha.tatashin@soleen… v5 https://lore.kernel.org/lkml/20210119043920.155044-1-pasha.tatashin@soleen.… v6 https://lore.kernel.org/lkml/20210120014333.222547-1-pasha.tatashin@soleen.… v7 https://lore.kernel.org/lkml/20210122033748.924330-1-pasha.tatashin@soleen.… Pavel Tatashin (14): mm/gup: don't pin migrated cma pages in movable zone mm/gup: check every subpage of a compound page during isolation mm/gup: return an error on migration failure mm/gup: check for isolation errors mm cma: rename PF_MEMALLOC_NOCMA to PF_MEMALLOC_PIN mm: apply per-task gfp constraints in fast path mm: honor PF_MEMALLOC_PIN for all movable pages mm/gup: do not migrate zero page mm/gup: migrate pinned pages out of movable zone memory-hotplug.rst: add a note about ZONE_MOVABLE and page pinning mm/gup: change index type to long as it counts pages mm/gup: longterm pin migration cleanup selftests/vm: gup_test: fix test flag selftests/vm: gup_test: test faulting in kernel, and verify pinnable pages .../admin-guide/mm/memory-hotplug.rst | 9 + include/linux/migrate.h | 1 + include/linux/mm.h | 12 ++ include/linux/mmzone.h | 13 +- include/linux/pgtable.h | 3 +- include/linux/sched.h | 2 +- include/linux/sched/mm.h | 27 +-- include/trace/events/migrate.h | 3 +- mm/gup.c | 174 ++++++++---------- mm/gup_test.c | 29 +-- mm/gup_test.h | 3 +- mm/hugetlb.c | 4 +- mm/page_alloc.c | 33 ++-- tools/testing/selftests/vm/gup_test.c | 36 +++- 14 files changed, 190 insertions(+), 159 deletions(-) -- 2.25.1

4 years, 5 months

4
23
0 0

[PATCH v5 5/5] selftest/x86/signal: Include test cases for validating sigaltstack

by Chang S. Bae

The test measures the kernel's signal delivery with different (enough vs. insufficient) stack sizes. Signed-off-by: Chang S. Bae <chang.seok.bae(a)intel.com> Reviewed-by: Len Brown <len.brown(a)intel.com> Cc: Borislav Petkov <bp(a)alien8.de> Cc: x86(a)kernel.org Cc: linux-kselftest(a)vger.kernel.org Cc: linux-kernel(a)vger.kernel.org --- Changes from v3: * Revised test messages again (Borislav Petkov) Changes from v2: * Revised test messages (Borislav Petkov) --- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/sigaltstack.c | 128 ++++++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/sigaltstack.c diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 333980375bc7..65bba2ae86ee 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -13,7 +13,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_vsyscall mov_ss_trap \ - syscall_arg_fault fsgsbase_restore + syscall_arg_fault fsgsbase_restore sigaltstack TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer diff --git a/tools/testing/selftests/x86/sigaltstack.c b/tools/testing/selftests/x86/sigaltstack.c new file mode 100644 index 000000000000..f689af75e979 --- /dev/null +++ b/tools/testing/selftests/x86/sigaltstack.c @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#define _GNU_SOURCE +#include <signal.h> +#include <stdio.h> +#include <stdbool.h> +#include <string.h> +#include <err.h> +#include <errno.h> +#include <limits.h> +#include <sys/mman.h> +#include <sys/auxv.h> +#include <sys/prctl.h> +#include <sys/resource.h> +#include <setjmp.h> + +/* sigaltstack()-enforced minimum stack */ +#define ENFORCED_MINSIGSTKSZ 2048 + +#ifndef AT_MINSIGSTKSZ +# define AT_MINSIGSTKSZ 51 +#endif + +static int nerrs; + +static bool sigalrm_expected; + +static unsigned long at_minstack_size; + +static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), + int flags) +{ + struct sigaction sa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_sigaction = handler; + sa.sa_flags = SA_SIGINFO | flags; + sigemptyset(&sa.sa_mask); + if (sigaction(sig, &sa, 0)) + err(1, "sigaction"); +} + +static void clearhandler(int sig) +{ + struct sigaction sa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigemptyset(&sa.sa_mask); + if (sigaction(sig, &sa, 0)) + err(1, "sigaction"); +} + +static int setup_altstack(void *start, unsigned long size) +{ + stack_t ss; + + memset(&ss, 0, sizeof(ss)); + ss.ss_size = size; + ss.ss_sp = start; + + return sigaltstack(&ss, NULL); +} + +static jmp_buf jmpbuf; + +static void sigsegv(int sig, siginfo_t *info, void *ctx_void) +{ + if (sigalrm_expected) { + printf("[FAIL]\tWrong signal delivered: SIGSEGV (expected SIGALRM)."); + nerrs++; + } else { + printf("[OK]\tSIGSEGV signal delivered.\n"); + } + + siglongjmp(jmpbuf, 1); +} + +static void sigalrm(int sig, siginfo_t *info, void *ctx_void) +{ + if (!sigalrm_expected) { + printf("[FAIL]\tWrong signal delivered: SIGALRM (expected SIGSEGV)."); + nerrs++; + } else { + printf("[OK]\tSIGALRM signal delivered.\n"); + } +} + +static void test_sigaltstack(void *altstack, unsigned long size) +{ + if (setup_altstack(altstack, size)) + err(1, "sigaltstack()"); + + sigalrm_expected = (size > at_minstack_size) ? true : false; + + sethandler(SIGSEGV, sigsegv, 0); + sethandler(SIGALRM, sigalrm, SA_ONSTACK); + + if (!sigsetjmp(jmpbuf, 1)) { + printf("[RUN]\tTest an alternate signal stack of %ssufficient size.\n", + sigalrm_expected ? "" : "in"); + printf("\tRaise SIGALRM. %s is expected to be delivered.\n", + sigalrm_expected ? "It" : "SIGSEGV"); + raise(SIGALRM); + } + + clearhandler(SIGALRM); + clearhandler(SIGSEGV); +} + +int main(void) +{ + void *altstack; + + at_minstack_size = getauxval(AT_MINSIGSTKSZ); + + altstack = mmap(NULL, at_minstack_size + SIGSTKSZ, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (altstack == MAP_FAILED) + err(1, "mmap()"); + + if ((ENFORCED_MINSIGSTKSZ + 1) < at_minstack_size) + test_sigaltstack(altstack, ENFORCED_MINSIGSTKSZ + 1); + + test_sigaltstack(altstack, at_minstack_size + SIGSTKSZ); + + return nerrs == 0 ? 0 : 1; +} -- 2.17.1

4 years, 5 months

1
0
0 0

[PATCH] selftests/powerpc: make the test check in eeh-basic.sh posix compliant

by Po-Hsu Lin

The == operand is a bash extension, thus this will fail on Ubuntu with As the /bin/sh on Ubuntu is pointed to DASH. Use -eq to fix this posix compatibility issue. Fixes: 996f9e0f93f162 ("selftests/powerpc: Fix eeh-basic.sh exit codes") Signed-off-by: Po-Hsu Lin <po-hsu.lin(a)canonical.com> --- tools/testing/selftests/powerpc/eeh/eeh-basic.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/eeh/eeh-basic.sh b/tools/testing/selftests/powerpc/eeh/eeh-basic.sh index 0d783e1..64779f0 100755 --- a/tools/testing/selftests/powerpc/eeh/eeh-basic.sh +++ b/tools/testing/selftests/powerpc/eeh/eeh-basic.sh @@ -86,5 +86,5 @@ echo "$failed devices failed to recover ($dev_count tested)" lspci | diff -u $pre_lspci - rm -f $pre_lspci -test "$failed" == 0 +test "$failed" -eq 0 exit $? -- 2.7.4

4 years, 5 months

2
1
0 0

[PATCH v9 00/14] prohibit pinning pages in ZONE_MOVABLE

by Pavel Tatashin

Changelog --------- v9 - Renamed gpf_to_alloc_flags() to gfp_to_alloc_flags_cma(); thanks Lecopzer Chen for noticing. - Fixed warning reported scripts/checkpatch.pl: "Logical continuations should be on the previous line" v8 - Added reviewed by's from John Hubbard - Fixed subjects for selftests patches - Moved zero page check inside is_pinnable_page() as requested by Jason Gunthorpe. v7 - Added reviewed-by's - Fixed a compile bug on non-mmu builds reported by robot v6 Small update, but I wanted to send it out quicker, as it removes a controversial patch and replaces it with something sane. - Removed forcing FOLL_WRITE for longterm gup, instead added a patch to skip zero pages during migration. - Added reviewed-by's and minor log changes. v5 - Added the following patches to the beginning of series, which are fixes to the other existing problems with CMA migration code: mm/gup: check every subpage of a compound page during isolation mm/gup: return an error on migration failure mm/gup: check for isolation errors also at the beginning of series mm/gup: do not allow zero page for pinned pages - remove .gfp_mask/.reclaim_idx changes from mm/vmscan.c - update movable zone header comment in patch 8 instead of patch 3, fix the comment - Added acked, sign-offs - Updated commit logs based on feedback - Addressed issues reported by Michal and Jason. - Remove: #define PINNABLE_MIGRATE_MAX 10 #define PINNABLE_ISOLATE_MAX 100 Instead: fail on the first migration failure, and retry isolation forever as their failures are transient. - In self-set addressed some of the comments from John Hubbard, updated commit logs, and added comments. Renamed gup->flags with gup->test_flags. v4 - Address page migration comments. New patch: mm/gup: limit number of gup migration failures, honor failures Implements the limiting number of retries for migration failures, and also check for isolation failures. Added a test case into gup_test to verify that pages never long-term pinned in a movable zone, and also added tests to fault both in kernel and in userland. v3 - Merged with linux-next, which contains clean-up patch from Jason, therefore this series is reduced by two patches which did the same thing. v2 - Addressed all review comments - Added Reviewed-by's. - Renamed PF_MEMALLOC_NOMOVABLE to PF_MEMALLOC_PIN - Added is_pinnable_page() to check if page can be longterm pinned - Fixed gup fast path by checking is_in_pinnable_zone() - rename cma_page_list to movable_page_list - add a admin-guide note about handling pinned pages in ZONE_MOVABLE, updated caveat about pinned pages from linux/mmzone.h - Move current_gfp_context() to fast-path --------- When page is pinned it cannot be moved and its physical address stays the same until pages is unpinned. This is useful functionality to allows userland to implementation DMA access. For example, it is used by vfio in vfio_pin_pages(). However, this functionality breaks memory hotplug/hotremove assumptions that pages in ZONE_MOVABLE can always be migrated. This patch series fixes this issue by forcing new allocations during page pinning to omit ZONE_MOVABLE, and also to migrate any existing pages from ZONE_MOVABLE during pinning. It uses the same scheme logic that is currently used by CMA, and extends the functionality for all allocations. For more information read the discussion [1] about this problem. [1] https://lore.kernel.org/lkml/CA+CK2bBffHBxjmb9jmSKacm0fJMinyt3Nhk8Nx6iudcQS… Previous versions: v1 https://lore.kernel.org/lkml/20201202052330.474592-1-pasha.tatashin@soleen.… v2 https://lore.kernel.org/lkml/20201210004335.64634-1-pasha.tatashin@soleen.c… v3 https://lore.kernel.org/lkml/20201211202140.396852-1-pasha.tatashin@soleen.… v4 https://lore.kernel.org/lkml/20201217185243.3288048-1-pasha.tatashin@soleen… v5 https://lore.kernel.org/lkml/20210119043920.155044-1-pasha.tatashin@soleen.… v6 https://lore.kernel.org/lkml/20210120014333.222547-1-pasha.tatashin@soleen.… v7 https://lore.kernel.org/lkml/20210122033748.924330-1-pasha.tatashin@soleen.… v8 https://lore.kernel.org/lkml/20210125194751.1275316-1-pasha.tatashin@soleen… Pavel Tatashin (14): mm/gup: don't pin migrated cma pages in movable zone mm/gup: check every subpage of a compound page during isolation mm/gup: return an error on migration failure mm/gup: check for isolation errors mm cma: rename PF_MEMALLOC_NOCMA to PF_MEMALLOC_PIN mm: apply per-task gfp constraints in fast path mm: honor PF_MEMALLOC_PIN for all movable pages mm/gup: do not migrate zero page mm/gup: migrate pinned pages out of movable zone memory-hotplug.rst: add a note about ZONE_MOVABLE and page pinning mm/gup: change index type to long as it counts pages mm/gup: longterm pin migration cleanup selftests/vm: gup_test: fix test flag selftests/vm: gup_test: test faulting in kernel, and verify pinnable pages .../admin-guide/mm/memory-hotplug.rst | 9 + include/linux/migrate.h | 1 + include/linux/mm.h | 12 ++ include/linux/mmzone.h | 13 +- include/linux/pgtable.h | 3 +- include/linux/sched.h | 2 +- include/linux/sched/mm.h | 27 +-- include/trace/events/migrate.h | 3 +- mm/gup.c | 174 ++++++++---------- mm/gup_test.c | 29 +-- mm/gup_test.h | 3 +- mm/hugetlb.c | 4 +- mm/page_alloc.c | 33 ++-- tools/testing/selftests/vm/gup_test.c | 36 +++- 14 files changed, 190 insertions(+), 159 deletions(-) -- 2.25.1

4 years, 5 months

1
14
0 0

Re: [PATCH v3 1/5] selftests/x86: Simplify the code to get vdso base address in sgx

by Jarkko Sakkinen

What the short summary is saying now, is that this commit would make the existing code to use vDSO base address. It's already doing that. You could instead just "Use getauxval() to simplify the code". Also, I'd prefer to properly use upper and lower case letter, e.g. vDSO instead of vdso. Reply-To: In-Reply-To: <20210124062907.88229-2-tianjia.zhang(a)linux.alibaba.com> On Sun, Jan 24, 2021 at 02:29:03PM +0800, Tianjia Zhang wrote: > This patch uses the library function `getauxval(AT_SYSINFO_EHDR)` > instead of the custom function `vdso_get_base_addr` to obtain the Use either double or single quotation mark instead of hyphen. > base address of vDSO, which will simplify the code implementation. > > Signed-off-by: Tianjia Zhang <tianjia.zhang(a)linux.alibaba.com> This needs to be imperative form, e.g. "Simplify the code implemntation by using getauxval() instead of a custom function." > --- > tools/testing/selftests/sgx/main.c | 24 ++++-------------------- > 1 file changed, 4 insertions(+), 20 deletions(-) > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > index 724cec700926..365d01dea67b 100644 > --- a/tools/testing/selftests/sgx/main.c > +++ b/tools/testing/selftests/sgx/main.c > @@ -15,6 +15,7 @@ > #include <sys/stat.h> > #include <sys/time.h> > #include <sys/types.h> > +#include <sys/auxv.h> > #include "defines.h" > #include "main.h" > #include "../kselftest.h" > @@ -28,24 +29,6 @@ struct vdso_symtab { > Elf64_Word *elf_hashtab; > }; > > -static void *vdso_get_base_addr(char *envp[]) > -{ > - Elf64_auxv_t *auxv; > - int i; > - > - for (i = 0; envp[i]; i++) > - ; > - > - auxv = (Elf64_auxv_t *)&envp[i + 1]; > - > - for (i = 0; auxv[i].a_type != AT_NULL; i++) { > - if (auxv[i].a_type == AT_SYSINFO_EHDR) > - return (void *)auxv[i].a_un.a_val; > - } > - > - return NULL; > -} > - > static Elf64_Dyn *vdso_get_dyntab(void *addr) > { > Elf64_Ehdr *ehdr = addr; > @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r > return 0; > } > > -int main(int argc, char *argv[], char *envp[]) > +int main(int argc, char *argv[]) > { > struct sgx_enclave_run run; > struct vdso_symtab symtab; > @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) > memset(&run, 0, sizeof(run)); > run.tcs = encl.encl_base; > > - addr = vdso_get_base_addr(envp); > + /* Get vDSO base address */ > + addr = (void *)(uintptr_t)getauxval(AT_SYSINFO_EHDR); You could just case the result the result directly to void *. > if (!addr) > goto err; > > -- > 2.19.1.3.ge56e4f7 > > /Jarkko

4 years, 5 months

2
1
0 0

[PATCH net-next v4 0/2] Add nci suit and virtual nci device driver

by Bongsu Jeon

From: Bongsu Jeon <bongsu.jeon(a)samsung.com> 1/2 is the Virtual NCI device driver. 2/2 is the NCI selftest suite v4: 1/2 - flip the condition for the ioctl. - refactor some code. - remove the unused function after refactoring. v3: 1/2 - change the Kconfig help comment. - remove the mutex init code. - remove the unnecessary mutex(nci_send_mutex). - remove the full_txbuff. - add the code to release skb at error case. - refactor some code. v2: 1/2 - change the permission of the Virtual NCI device. - add the ioctl to find the nci device index. 2/2 - add the NCI selftest suite. Bongsu Jeon (2): nfc: Add a virtual nci device driver selftests: Add nci suite MAINTAINERS | 8 + drivers/nfc/Kconfig | 11 + drivers/nfc/Makefile | 1 + drivers/nfc/virtual_ncidev.c | 215 +++++++++ tools/testing/selftests/Makefile | 1 + tools/testing/selftests/nci/Makefile | 6 + tools/testing/selftests/nci/config | 3 + tools/testing/selftests/nci/nci_dev.c | 599 ++++++++++++++++++++++++++ 8 files changed, 844 insertions(+) create mode 100644 drivers/nfc/virtual_ncidev.c create mode 100644 tools/testing/selftests/nci/Makefile create mode 100644 tools/testing/selftests/nci/config create mode 100644 tools/testing/selftests/nci/nci_dev.c -- 2.25.1

4 years, 5 months

2
3
0 0

[RFC 0/3] kunit vs structleak

by Arnd Bergmann

From: Arnd Bergmann <arnd(a)arndb.de> I ran into a couple of problems with kunit tests taking too much stack space, sometimes dangerously so. These the the three instances that cause an increase over the warning limit of some architectures: lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] Ideally there should be a way to rewrite the kunit infrastructure that avoids the explosion of stack data when the structleak plugin is used. A rather drastic measure would be to use Kconfig logic to make the two options mutually exclusive. This would clearly work, but is probably not needed. As a simpler workaround, this disables the plugin for the three files in which the excessive stack usage was observed. Arnd Arnd Bergmann (3): bitfield: build kunit tests without structleak plugin drivers/base: build kunit tests without structleak plugin thunderbolt: build kunit tests without structleak plugin drivers/base/test/Makefile | 1 + drivers/thunderbolt/Makefile | 1 + lib/Makefile | 1 + 3 files changed, 3 insertions(+) Cc: Kees Cook <keescook(a)chromium.org> Cc: Brendan Higgins <brendanhiggins(a)google.com> Cc: Shuah Khan <skhan(a)linuxfoundation.org> Cc: Geert Uytterhoeven <geert+renesas(a)glider.be> Cc: Alan Maguire <alan.maguire(a)oracle.com> Cc: Dmitry Torokhov <dmitry.torokhov(a)gmail.com> Cc: Mika Westerberg <mika.westerberg(a)linux.intel.com> Cc: Vitor Massaru Iha <vitor(a)massaru.org> Cc: linux-hardening(a)vger.kernel.org Cc: linux-kselftest(a)vger.kernel.org Cc: kunit-dev(a)googlegroups.com Cc: linux-kernel(a)vger.kernel.org -- 2.29.2

4 years, 5 months

4
6
0 0

[PATCH] kunit: don't show `1 == 1` in failed assertion messages

by Daniel Latypov

Currently, given something (fairly dystopian) like > KUNIT_EXPECT_EQ(test, 2 + 2, 5) KUnit will prints a failure message like this. > Expected 2 + 2 == 5, but > 2 + 2 == 4 > 5 == 5 With this patch, the output just becomes > Expected 2 + 2 == 5, but > 2 + 2 == 4 This patch is slightly hacky, but it's quite common* to compare an expression to a literal integer value, so this can make KUnit less chatty in many cases. (This patch also fixes variants like KUNIT_EXPECT_GT, LE, et al.). It also allocates an additional string briefly, but given this only happens on test failures, it doesn't seem too bad a tradeoff. Also, in most cases it'll realize the lengths are unequal and bail out before the allocation. We could save the result of the formatted string to avoid wasting this extra work, but it felt cleaner to leave it as-is. Edge case: for something silly and unrealistic like > KUNIT_EXPECT_EQ(test, 4, 5); It'll generate this message with a trailing "but" > Expected 2 + 2 == 5, but > <next line of normal output> It didn't feel worth adding a check up-front to see if both sides are literals to handle this better. *A quick grep suggests 100+ comparisons to an integer literal as the right hand side. Signed-off-by: Daniel Latypov <dlatypov(a)google.com> --- lib/kunit/assert.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 33acdaa28a7d..e0ec7d6fed6f 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -85,6 +85,29 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, } EXPORT_SYMBOL_GPL(kunit_ptr_not_err_assert_format); +/* Checks if `text` is a literal representing `value`, e.g. "5" and 5 */ +static bool is_literal(struct kunit *test, const char *text, long long value, + gfp_t gfp) +{ + char *buffer; + int len; + bool ret; + + len = snprintf(NULL, 0, "%lld", value); + if (strlen(text) != len) + return false; + + buffer = kunit_kmalloc(test, len+1, gfp); + if (!buffer) + return false; + + snprintf(buffer, len+1, "%lld", value); + ret = strncmp(buffer, text, len) == 0; + + kunit_kfree(test, buffer); + return ret; +} + void kunit_binary_assert_format(const struct kunit_assert *assert, struct string_stream *stream) { @@ -97,12 +120,16 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, binary_assert->left_text, binary_assert->operation, binary_assert->right_text); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld\n", - binary_assert->left_text, - binary_assert->left_value); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld", - binary_assert->right_text, - binary_assert->right_value); + if (!is_literal(stream->test, binary_assert->left_text, + binary_assert->left_value, stream->gfp)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld\n", + binary_assert->left_text, + binary_assert->left_value); + if (!is_literal(stream->test, binary_assert->right_text, + binary_assert->right_value, stream->gfp)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld", + binary_assert->right_text, + binary_assert->right_value); kunit_assert_print_msg(assert, stream); } EXPORT_SYMBOL_GPL(kunit_binary_assert_format); base-commit: e5ff2cb9cf67a542f2ec7fb87e24934c88b32678 -- 2.30.0.365.g02bc693789-goog

4 years, 5 months

3
3
0 0

[PATCH] kunit: make kunit_tool accept optional path to .kunitconfig fragment

by Daniel Latypov

Currently running tests via KUnit tool means tweaking a .kunitconfig file, which you'd keep around locally and never commit. This changes makes it so users can pass in a path to a kunitconfig. One of the imagined use cases is having kunitconfig fragments in-tree to formalize interesting sets of tests for features/subsystems, e.g. $ ./tools/testing/kunit/kunit.py run fs/ext4/kunitconfig For now, this hypothetical fs/ext4/kunitconfig would contain CONFIG_KUNIT=y CONFIG_EXT4_FS=y CONFIG_EXT4_KUNIT_TESTS=y At the moment, it's not hard to manually whip up this file, but as more and more tests get added, this will get tedious. It also opens the door to documenting how to run all the tests relevant to a specific subsystem or feature as a simple one-liner. This can be seen as an analogue to tools/testing/selftests/*/config But in the case of KUnit, the tests live in the same directory as the code-under-test, so it feels more natural to allow the kunitconfig fragments to live anywhere. (Though, people could create a separate directory if wanted; this patch imposes no restrictions on the path). Signed-off-by: Daniel Latypov <dlatypov(a)google.com> --- tools/testing/kunit/kunit.py | 9 ++++++--- tools/testing/kunit/kunit_kernel.py | 12 ++++++++---- tools/testing/kunit/kunit_tool_test.py | 25 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index e808a47c839b..3204a23bd16e 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -188,6 +188,9 @@ def add_build_opts(parser) -> None: help='As in the make command, "Specifies the number of ' 'jobs (commands) to run simultaneously."', type=int, default=8, metavar='jobs') + parser.add_argument('kunitconfig', + help='Path to Kconfig fragment that enables KUnit tests', + type=str, nargs='?', metavar='kunitconfig') def add_exec_opts(parser) -> None: parser.add_argument('--timeout', @@ -256,7 +259,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir) if not linux: - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir) + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig) request = KunitRequest(cli_args.raw_output, cli_args.timeout, @@ -274,7 +277,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir) if not linux: - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir) + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig) request = KunitConfigRequest(cli_args.build_dir, cli_args.make_options) @@ -286,7 +289,7 @@ def main(argv, linux=None): sys.exit(1) elif cli_args.subcommand == 'build': if not linux: - linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir) + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig) request = KunitBuildRequest(cli_args.jobs, cli_args.build_dir, diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 2076a5a2d060..0b461663e7d9 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -123,7 +123,7 @@ def get_outfile_path(build_dir) -> str: class LinuxSourceTree(object): """Represents a Linux kernel source tree with KUnit tests.""" - def __init__(self, build_dir: str, load_config=True, defconfig=DEFAULT_KUNITCONFIG_PATH) -> None: + def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None: signal.signal(signal.SIGINT, self.signal_handler) self._ops = LinuxSourceTreeOperations() @@ -131,9 +131,13 @@ class LinuxSourceTree(object): if not load_config: return - kunitconfig_path = get_kunitconfig_path(build_dir) - if not os.path.exists(kunitconfig_path): - shutil.copyfile(defconfig, kunitconfig_path) + if kunitconfig_path: + if not os.path.exists(kunitconfig_path): + raise ConfigError(f'Specified kunitconfig ({kunitconfig_path}) does not exist') + else: + kunitconfig_path = get_kunitconfig_path(build_dir) + if not os.path.exists(kunitconfig_path): + shutil.copyfile(DEFAULT_KUNITCONFIG_PATH, kunitconfig_path) self._kconfig = kunit_config.Kconfig() self._kconfig.read_from_file(kunitconfig_path) diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index b593f4448e83..533fe41b5123 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -12,6 +12,7 @@ from unittest import mock import tempfile, shutil # Handling test_tmpdir import json +import signal import os import kunit_config @@ -250,6 +251,23 @@ class KUnitParserTest(unittest.TestCase): result.status) self.assertEqual('kunit-resource-test', result.suites[0].name) +class LinuxSourceTreeTest(unittest.TestCase): + + def setUp(self): + mock.patch.object(signal, 'signal').start() + self.addCleanup(mock.patch.stopall) + + def test_invalid_kunitconfig(self): + with self.assertRaisesRegex(kunit_kernel.ConfigError, 'nonexistent.* does not exist'): + kunit_kernel.LinuxSourceTree('', kunitconfig_path='/nonexistent_file') + + def test_valid_kunitconfig(self): + with tempfile.NamedTemporaryFile('wt') as kunitconfig: + tree = kunit_kernel.LinuxSourceTree('', kunitconfig_path=kunitconfig.name) + + # TODO: add more test cases. + + class KUnitJsonTest(unittest.TestCase): def _json_for(self, log_file): @@ -399,5 +417,12 @@ class KUnitMainTest(unittest.TestCase): self.linux_source_mock.run_kernel.assert_called_once_with(build_dir=build_dir, timeout=300) self.print_mock.assert_any_call(StrContains('Testing complete.')) + @mock.patch.object(kunit_kernel, 'LinuxSourceTree') + def test_run_kunitconfig(self, mock_linux_init): + mock_linux_init.return_value = self.linux_source_mock + kunit.main(['run', 'mykunitconfig']) + # Just verify that we parsed and initialized it correctly here. + mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig') + if __name__ == '__main__': unittest.main() base-commit: 2b8fdbbf1c616300312f71fe5b21fe8f03129950 -- 2.30.0.280.ga3ce27912f-goog

4 years, 5 months

2
2
0 0

[PATCH bpf-next] tools: Factor Clang, LLC and LLVM utils definitions

by Sedat Dilek

When dealing with BPF/BTF/pahole and DWARF v5 I wanted to build bpftool. While looking into the source code I found duplicate assignments in misc tools for the LLVM eco system, e.g. clang and llvm-objcopy. Move the Clang, LLC and/or LLVM utils definitions to tools/scripts/Makefile.include file and add missing includes where needed. Honestly, I was inspired by commit c8a950d0d3b9 ("tools: Factor HOSTCC, HOSTLD, HOSTAR definitions"). I tested with bpftool and perf on Debian/testing AMD64 and LLVM/Clang v11.1.0-rc1. Build instructions: [ make and make-options ] MAKE="make V=1" MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang LD=ld.lld LLVM=1 LLVM_IAS=1" MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole" [ clean-up ] $MAKE $MAKE_OPTS -C tools/ clean [ bpftool ] $MAKE $MAKE_OPTS -C tools/bpf/bpftool/ [ perf ] PYTHON=python3 $MAKE $MAKE_OPTS -C tools/perf/ I was careful with respecting the user's wish to override custom compiler, linker, GNU/binutils and/or LLVM utils settings. Some personal notes: 1. I have NOT tested with cross-toolchain for other archs (cross compiler/linker etc.). 2. This patch is on top of bpf-next. CC: bpf(a)vger.kernel.org Acked-by: Andrii Nakryiko <andrii(a)kernel.org> Acked-by: Jiri Olsa <jolsa(a)redhat.com> # tools/build and tools/perf Signed-off-by: Sedat Dilek <sedat.dilek(a)gmail.com> --- tools/bpf/bpftool/Makefile | 2 -- tools/bpf/runqslower/Makefile | 3 --- tools/build/feature/Makefile | 4 ++-- tools/perf/Makefile.perf | 1 - tools/scripts/Makefile.include | 7 +++++++ tools/testing/selftests/bpf/Makefile | 2 -- tools/testing/selftests/tc-testing/Makefile | 3 +-- 7 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 45ac2f9e0aa9..8ced1655fea6 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -75,8 +75,6 @@ endif INSTALL ?= install RM ?= rm -f -CLANG ?= clang -LLVM_STRIP ?= llvm-strip FEATURE_USER = .bpftool FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib libcap \ diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile index 4d5ca54fcd4c..9d9fb6209be1 100644 --- a/tools/bpf/runqslower/Makefile +++ b/tools/bpf/runqslower/Makefile @@ -3,9 +3,6 @@ include ../../scripts/Makefile.include OUTPUT ?= $(abspath .output)/ -CLANG ?= clang -LLC ?= llc -LLVM_STRIP ?= llvm-strip BPFTOOL_OUTPUT := $(OUTPUT)bpftool/ DEFAULT_BPFTOOL := $(BPFTOOL_OUTPUT)bpftool BPFTOOL ?= $(DEFAULT_BPFTOOL) diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 89ba522e377d..3e55edb3ea54 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -1,4 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +include ../../scripts/Makefile.include + FILES= \ test-all.bin \ test-backtrace.bin \ @@ -76,8 +78,6 @@ FILES= \ FILES := $(addprefix $(OUTPUT),$(FILES)) PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config -LLVM_CONFIG ?= llvm-config -CLANG ?= clang all: $(FILES) diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 62f3deb1d3a8..f4df7534026d 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -176,7 +176,6 @@ endef LD += $(EXTRA_LDFLAGS) PKG_CONFIG = $(CROSS_COMPILE)pkg-config -LLVM_CONFIG ?= llvm-config RM = rm -f LN = ln -f diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include index 1358e89cdf7d..4255e71f72b7 100644 --- a/tools/scripts/Makefile.include +++ b/tools/scripts/Makefile.include @@ -69,6 +69,13 @@ HOSTCC ?= gcc HOSTLD ?= ld endif +# Some tools require Clang, LLC and/or LLVM utils +CLANG ?= clang +LLC ?= llc +LLVM_CONFIG ?= llvm-config +LLVM_OBJCOPY ?= llvm-objcopy +LLVM_STRIP ?= llvm-strip + ifeq ($(CC_NO_CLANG), 1) EXTRA_WARNINGS += -Wstrict-aliasing=3 endif diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 63d6288e419c..f0674d406f40 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -19,8 +19,6 @@ ifneq ($(wildcard $(GENHDR)),) GENFLAGS := -DHAVE_GENHDR endif -CLANG ?= clang -LLVM_OBJCOPY ?= llvm-objcopy BPF_GCC ?= $(shell command -v bpf-gcc;) SAN_CFLAGS ?= CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS) $(SAN_CFLAGS) \ diff --git a/tools/testing/selftests/tc-testing/Makefile b/tools/testing/selftests/tc-testing/Makefile index 91fee5c43274..4d639279f41e 100644 --- a/tools/testing/selftests/tc-testing/Makefile +++ b/tools/testing/selftests/tc-testing/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +include ../../../scripts/Makefile.include top_srcdir = $(abspath ../../../..) APIDIR := $(top_scrdir)/include/uapi @@ -7,8 +8,6 @@ TEST_GEN_FILES = action.o KSFT_KHDR_INSTALL := 1 include ../lib.mk -CLANG ?= clang -LLC ?= llc PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1) ifeq ($(PROBE),) -- 2.30.0

4 years, 5 months

2
1
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror