From b2c93e300a11b419b4c67ce08e16fa1436d8118c Mon Sep 17 00:00:00 2001 From: Anders Roxell Date: Tue, 6 Feb 2018 16:23:39 -0600 Subject: selftests: sync: missing CFLAGS while compiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on patch: https://patchwork.kernel.org/patch/10042045/ arch64-linux-gnu-gcc -c sync.c -o sync/sync.o sync.c:42:29: fatal error: linux/sync_file.h: No such file or directory #include ^ CFLAGS is not used during the compile step, so the system instead of kernel headers are used. Fix this by adding CFLAGS to the OBJS compile rule. Reported-by: Lei Yang Signed-off-by: Anders Roxell Signed-off-by: Daniel Díaz Signed-off-by: Shuah Khan --- tools/testing/selftests/sync/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile index b3c8ba3cb668..d0121a8a3523 100644 --- a/tools/testing/selftests/sync/Makefile +++ b/tools/testing/selftests/sync/Makefile @@ -30,7 +30,7 @@ $(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS) $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS) $(OBJS): $(OUTPUT)/%.o: %.c - $(CC) -c $^ -o $@ + $(CC) -c $^ -o $@ $(CFLAGS) $(TESTS): $(OUTPUT)/%.o: %.c $(CC) -c $^ -o $@ -- cgit v1.2.3 From 70b574e7d719bdf96d26528cb289f3e782e83979 Mon Sep 17 00:00:00 2001 From: Dominik Brodowski Date: Sun, 11 Feb 2018 11:59:50 +0100 Subject: selftest/vDSO: fix O= The vDSO selftests ignored the O= or KBUILD_OUTPUT= parameters. Fix it. Signed-off-by: Dominik Brodowski Signed-off-by: Shuah Khan --- tools/testing/selftests/vDSO/Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 3d5a62ff7d31..f5d7a7851e21 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -1,4 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +include ../lib.mk + ifndef CROSS_COMPILE CFLAGS := -std=gnu99 CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector @@ -6,16 +8,14 @@ ifeq ($(CONFIG_X86_32),y) LDLIBS += -lgcc_s endif -TEST_PROGS := vdso_test vdso_standalone_test_x86 +TEST_PROGS := $(OUTPUT)/vdso_test $(OUTPUT)/vdso_standalone_test_x86 all: $(TEST_PROGS) -vdso_test: parse_vdso.c vdso_test.c -vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c +$(OUTPUT)/vdso_test: parse_vdso.c vdso_test.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 \ - -o vdso_standalone_test_x86 + -o $@ -include ../lib.mk -clean: - rm -fr $(TEST_PROGS) +EXTRA_CLEAN := $(TEST_PROGS) endif -- cgit v1.2.3 From 64136fb76039defd193e9e885bb722919d220021 Mon Sep 17 00:00:00 2001 From: Daniel Díaz Date: Tue, 6 Feb 2018 17:52:05 -0600 Subject: selftests/android: Fix line continuation in Makefile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Makefile lacks a couple of line continuation backslashes in an `if' clause, which can make the subsequent rsync command go awry over the whole filesystem (`rsync -a / /`). /bin/sh: -c: line 5: syntax error: unexpected end of file make[1]: [all] Error 1 (ignored) TEST=$DIR"_test.sh"; \ if [ -e $DIR/$TEST ]; then /bin/sh: -c: line 2: syntax error: unexpected end of file make[1]: [all] Error 1 (ignored) rsync -a $DIR/$TEST $BUILD_TARGET/; [...a myriad of:] [ rsync: readlink_stat("...") failed: Permission denied (13)] [ skipping non-regular file "..."] [ rsync: opendir "..." failed: Permission denied (13)] [and many other errors...] fi make[1]: fi: Command not found make[1]: [all] Error 127 (ignored) done make[1]: done: Command not found make[1]: [all] Error 127 (ignored) Signed-off-by: Daniel Díaz Acked-by: Pintu Agarwal Signed-off-by: Shuah Khan --- tools/testing/selftests/android/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/android/Makefile b/tools/testing/selftests/android/Makefile index 1a7492268993..f6304d2be90c 100644 --- a/tools/testing/selftests/android/Makefile +++ b/tools/testing/selftests/android/Makefile @@ -11,11 +11,11 @@ all: BUILD_TARGET=$(OUTPUT)/$$DIR; \ mkdir $$BUILD_TARGET -p; \ make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\ - #SUBDIR test prog name should be in the form: SUBDIR_test.sh + #SUBDIR test prog name should be in the form: SUBDIR_test.sh \ TEST=$$DIR"_test.sh"; \ - if [ -e $$DIR/$$TEST ]; then - rsync -a $$DIR/$$TEST $$BUILD_TARGET/; - fi + if [ -e $$DIR/$$TEST ]; then \ + rsync -a $$DIR/$$TEST $$BUILD_TARGET/; \ + fi \ done override define RUN_TESTS -- cgit v1.2.3 From 9a379e77033f02c4a071891afdf0f0a01eff8ccb Mon Sep 17 00:00:00 2001 From: Naresh Kamboju Date: Wed, 7 Feb 2018 14:47:20 +0530 Subject: selftests: pstore: Adding config fragment CONFIG_PSTORE_RAM=m pstore_tests and pstore_post_reboot_tests need CONFIG_PSTORE_RAM=m Signed-off-by: Naresh Kamboju Acked-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/pstore/config | 1 + 1 file changed, 1 insertion(+) (limited to 'tools') diff --git a/tools/testing/selftests/pstore/config b/tools/testing/selftests/pstore/config index 6a8e5a9bfc10..d148f9f89fb6 100644 --- a/tools/testing/selftests/pstore/config +++ b/tools/testing/selftests/pstore/config @@ -2,3 +2,4 @@ CONFIG_MISC_FILESYSTEMS=y CONFIG_PSTORE=y CONFIG_PSTORE_PMSG=y CONFIG_PSTORE_CONSOLE=y +CONFIG_PSTORE_RAM=m -- cgit v1.2.3 From 9a606f8d55cfc932ec02172aaed4124fdc150047 Mon Sep 17 00:00:00 2001 From: Anders Roxell Date: Tue, 6 Feb 2018 16:20:44 -0600 Subject: selftests: memfd: add config fragment for fuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memfd test requires to insert the fuse module (CONFIG_FUSE_FS). Signed-off-by: Anders Roxell Signed-off-by: Daniel Díaz Signed-off-by: Shuah Khan --- tools/testing/selftests/memfd/config | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/testing/selftests/memfd/config (limited to 'tools') diff --git a/tools/testing/selftests/memfd/config b/tools/testing/selftests/memfd/config new file mode 100644 index 000000000000..835c7f4dadcd --- /dev/null +++ b/tools/testing/selftests/memfd/config @@ -0,0 +1 @@ +CONFIG_FUSE_FS=m -- cgit v1.2.3 From ca36960211eb228bcbc7aaebfa0d027368a94c60 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 23 Feb 2018 22:29:05 +0100 Subject: bpf: allow xadd only on aligned memory The requirements around atomic_add() / atomic64_add() resp. their JIT implementations differ across architectures. E.g. while x86_64 seems just fine with BPF's xadd on unaligned memory, on arm64 it triggers via interpreter but also JIT the following crash: [ 830.864985] Unable to handle kernel paging request at virtual address ffff8097d7ed6703 [...] [ 830.916161] Internal error: Oops: 96000021 [#1] SMP [ 830.984755] CPU: 37 PID: 2788 Comm: test_verifier Not tainted 4.16.0-rc2+ #8 [ 830.991790] Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.29 07/17/2017 [ 830.998998] pstate: 80400005 (Nzcv daif +PAN -UAO) [ 831.003793] pc : __ll_sc_atomic_add+0x4/0x18 [ 831.008055] lr : ___bpf_prog_run+0x1198/0x1588 [ 831.012485] sp : ffff00001ccabc20 [ 831.015786] x29: ffff00001ccabc20 x28: ffff8017d56a0f00 [ 831.021087] x27: 0000000000000001 x26: 0000000000000000 [ 831.026387] x25: 000000c168d9db98 x24: 0000000000000000 [ 831.031686] x23: ffff000008203878 x22: ffff000009488000 [ 831.036986] x21: ffff000008b14e28 x20: ffff00001ccabcb0 [ 831.042286] x19: ffff0000097b5080 x18: 0000000000000a03 [ 831.047585] x17: 0000000000000000 x16: 0000000000000000 [ 831.052885] x15: 0000ffffaeca8000 x14: 0000000000000000 [ 831.058184] x13: 0000000000000000 x12: 0000000000000000 [ 831.063484] x11: 0000000000000001 x10: 0000000000000000 [ 831.068783] x9 : 0000000000000000 x8 : 0000000000000000 [ 831.074083] x7 : 0000000000000000 x6 : 000580d428000000 [ 831.079383] x5 : 0000000000000018 x4 : 0000000000000000 [ 831.084682] x3 : ffff00001ccabcb0 x2 : 0000000000000001 [ 831.089982] x1 : ffff8097d7ed6703 x0 : 0000000000000001 [ 831.095282] Process test_verifier (pid: 2788, stack limit = 0x0000000018370044) [ 831.102577] Call trace: [ 831.105012] __ll_sc_atomic_add+0x4/0x18 [ 831.108923] __bpf_prog_run32+0x4c/0x70 [ 831.112748] bpf_test_run+0x78/0xf8 [ 831.116224] bpf_prog_test_run_xdp+0xb4/0x120 [ 831.120567] SyS_bpf+0x77c/0x1110 [ 831.123873] el0_svc_naked+0x30/0x34 [ 831.127437] Code: 97fffe97 17ffffec 00000000 f9800031 (885f7c31) Reason for this is because memory is required to be aligned. In case of BPF, we always enforce alignment in terms of stack access, but not when accessing map values or packet data when the underlying arch (e.g. arm64) has CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS set. xadd on packet data that is local to us anyway is just wrong, so forbid this case entirely. The only place where xadd makes sense in fact are map values; xadd on stack is wrong as well, but it's been around for much longer. Specifically enforce strict alignment in case of xadd, so that we handle this case generically and avoid such crashes in the first place. Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)") Signed-off-by: Daniel Borkmann Signed-off-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 42 +++++++++++++-------- tools/testing/selftests/bpf/test_verifier.c | 58 +++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 16 deletions(-) (limited to 'tools') diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5fb69a85d967..c6eff108aa99 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1356,6 +1356,13 @@ static bool is_ctx_reg(struct bpf_verifier_env *env, int regno) return reg->type == PTR_TO_CTX; } +static bool is_pkt_reg(struct bpf_verifier_env *env, int regno) +{ + const struct bpf_reg_state *reg = cur_regs(env) + regno; + + return type_is_pkt_pointer(reg->type); +} + static int check_pkt_ptr_alignment(struct bpf_verifier_env *env, const struct bpf_reg_state *reg, int off, int size, bool strict) @@ -1416,10 +1423,10 @@ static int check_generic_ptr_alignment(struct bpf_verifier_env *env, } static int check_ptr_alignment(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, - int off, int size) + const struct bpf_reg_state *reg, int off, + int size, bool strict_alignment_once) { - bool strict = env->strict_alignment; + bool strict = env->strict_alignment || strict_alignment_once; const char *pointer_desc = ""; switch (reg->type) { @@ -1576,9 +1583,9 @@ static void coerce_reg_to_size(struct bpf_reg_state *reg, int size) * if t==write && value_regno==-1, some unknown value is stored into memory * if t==read && value_regno==-1, don't care what we read from memory */ -static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regno, int off, - int bpf_size, enum bpf_access_type t, - int value_regno) +static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regno, + int off, int bpf_size, enum bpf_access_type t, + int value_regno, bool strict_alignment_once) { struct bpf_reg_state *regs = cur_regs(env); struct bpf_reg_state *reg = regs + regno; @@ -1590,7 +1597,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn return size; /* alignment checks will add in reg->off themselves */ - err = check_ptr_alignment(env, reg, off, size); + err = check_ptr_alignment(env, reg, off, size, strict_alignment_once); if (err) return err; @@ -1735,21 +1742,23 @@ static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_ins return -EACCES; } - if (is_ctx_reg(env, insn->dst_reg)) { - verbose(env, "BPF_XADD stores into R%d context is not allowed\n", - insn->dst_reg); + if (is_ctx_reg(env, insn->dst_reg) || + is_pkt_reg(env, insn->dst_reg)) { + verbose(env, "BPF_XADD stores into R%d %s is not allowed\n", + insn->dst_reg, is_ctx_reg(env, insn->dst_reg) ? + "context" : "packet"); return -EACCES; } /* check whether atomic_add can read the memory */ err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off, - BPF_SIZE(insn->code), BPF_READ, -1); + BPF_SIZE(insn->code), BPF_READ, -1, true); if (err) return err; /* check whether atomic_add can write into the same memory */ return check_mem_access(env, insn_idx, insn->dst_reg, insn->off, - BPF_SIZE(insn->code), BPF_WRITE, -1); + BPF_SIZE(insn->code), BPF_WRITE, -1, true); } /* when register 'regno' is passed into function that will read 'access_size' @@ -2388,7 +2397,8 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn * is inferred from register state. */ for (i = 0; i < meta.access_size; i++) { - err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B, BPF_WRITE, -1); + err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B, + BPF_WRITE, -1, false); if (err) return err; } @@ -4632,7 +4642,7 @@ static int do_check(struct bpf_verifier_env *env) */ err = check_mem_access(env, insn_idx, insn->src_reg, insn->off, BPF_SIZE(insn->code), BPF_READ, - insn->dst_reg); + insn->dst_reg, false); if (err) return err; @@ -4684,7 +4694,7 @@ static int do_check(struct bpf_verifier_env *env) /* check that memory (dst_reg + off) is writeable */ err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off, BPF_SIZE(insn->code), BPF_WRITE, - insn->src_reg); + insn->src_reg, false); if (err) return err; @@ -4719,7 +4729,7 @@ static int do_check(struct bpf_verifier_env *env) /* check that memory (dst_reg + off) is writeable */ err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off, BPF_SIZE(insn->code), BPF_WRITE, - -1); + -1, false); if (err) return err; diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index c73592fa3d41..437c0b1c9d21 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -11163,6 +11163,64 @@ static struct bpf_test tests[] = { .result = REJECT, .prog_type = BPF_PROG_TYPE_TRACEPOINT, }, + { + "xadd/w check unaligned stack", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 1), + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_0, -8), + BPF_STX_XADD(BPF_W, BPF_REG_10, BPF_REG_0, -7), + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8), + BPF_EXIT_INSN(), + }, + .result = REJECT, + .errstr = "misaligned stack access off", + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + }, + { + "xadd/w check unaligned map", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, + BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), + BPF_EXIT_INSN(), + BPF_MOV64_IMM(BPF_REG_1, 1), + BPF_STX_XADD(BPF_W, BPF_REG_0, BPF_REG_1, 3), + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, 3), + BPF_EXIT_INSN(), + }, + .fixup_map1 = { 3 }, + .result = REJECT, + .errstr = "misaligned value access off", + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + }, + { + "xadd/w check unaligned pkt", + .insns = { + BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, + offsetof(struct xdp_md, data)), + BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, + offsetof(struct xdp_md, data_end)), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_2), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8), + BPF_JMP_REG(BPF_JLT, BPF_REG_1, BPF_REG_3, 2), + BPF_MOV64_IMM(BPF_REG_0, 99), + BPF_JMP_IMM(BPF_JA, 0, 0, 6), + BPF_MOV64_IMM(BPF_REG_0, 1), + BPF_ST_MEM(BPF_W, BPF_REG_2, 0, 0), + BPF_ST_MEM(BPF_W, BPF_REG_2, 3, 0), + BPF_STX_XADD(BPF_W, BPF_REG_2, BPF_REG_0, 1), + BPF_STX_XADD(BPF_W, BPF_REG_2, BPF_REG_0, 2), + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_2, 1), + BPF_EXIT_INSN(), + }, + .result = REJECT, + .errstr = "BPF_XADD stores into R2 packet", + .prog_type = BPF_PROG_TYPE_XDP, + }, }; static int probe_filter_length(const struct bpf_insn *fp) -- cgit v1.2.3 From faa312a543283c717342cd332b5b9247bd305dce Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 9 Jan 2018 13:27:01 +0100 Subject: tools/kvm_stat: simplify the sortkey function The 'sortkey' function references a value in its enclosing scope (closure). This is not common practice for a sort key function so let's replace it. Additionally, the function 'sorted' has already a parameter for reversing the result therefore the inversion of the values is unneeded. The check for stats[x][1] is also superfluous as it's ensured that this value is initialized with 0. Signed-off-by: Marc Hartmayer Tested-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index a5684d0968b4..d630f5f3e091 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1080,30 +1080,23 @@ class Tui(object): self.screen.move(row, 0) self.screen.clrtobot() stats = self.stats.get(self._display_guests) - - def sortCurAvg(x): - # sort by current events if available - if stats[x][1]: - return (-stats[x][1], -stats[x][0]) - else: - return (0, -stats[x][0]) - - def sortTotal(x): - # sort by totals - return (0, -stats[x][0]) total = 0. for key in stats.keys(): if key.find('(') is -1: total += stats[key][0] if self._sorting == SORT_DEFAULT: - sortkey = sortCurAvg + def sortkey((_k, v)): + # sort by (delta value, overall value) + return (v[1], v[0]) else: - sortkey = sortTotal + def sortkey((_k, v)): + # sort by overall value + return v[0] + tavg = 0 - for key in sorted(stats.keys(), key=sortkey): + for key, values in sorted(stats.items(), key=sortkey, reverse=True): if row >= self.screen.getmaxyx()[0] - 1: break - values = stats[key] if not values[0] and not values[1]: break if values[0] is not None: -- cgit v1.2.3 From 006f1548ac13d67d21865416a0f4e8062df1a85f Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 9 Jan 2018 13:27:02 +0100 Subject: tools/kvm_stat: use a namedtuple for storing the values Use a namedtuple for storing the values as it allows to access the fields of a tuple via names. This makes the overall code much easier to read and to understand. Access by index is still possible as before. Signed-off-by: Marc Hartmayer Tested-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index d630f5f3e091..2b7e83a5f7b8 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -33,7 +33,7 @@ import resource import struct import re import subprocess -from collections import defaultdict +from collections import defaultdict, namedtuple VMX_EXIT_REASONS = { 'EXCEPTION_NMI': 0, @@ -800,6 +800,9 @@ class DebugfsProvider(Provider): self.read(2) +EventStat = namedtuple('EventStat', ['value', 'delta']) + + class Stats(object): """Manages the data providers and the data they provide. @@ -867,10 +870,10 @@ class Stats(object): for provider in self.providers: new = provider.read(by_guest=by_guest) for key in new if by_guest else provider.fields: - oldval = self.values.get(key, (0, 0))[0] + oldval = self.values.get(key, EventStat(0, 0)).value newval = new.get(key, 0) newdelta = newval - oldval - self.values[key] = (newval, newdelta) + self.values[key] = EventStat(newval, newdelta) return self.values def toggle_display_guests(self, to_pid): @@ -1083,28 +1086,28 @@ class Tui(object): total = 0. for key in stats.keys(): if key.find('(') is -1: - total += stats[key][0] + total += stats[key].value if self._sorting == SORT_DEFAULT: def sortkey((_k, v)): # sort by (delta value, overall value) - return (v[1], v[0]) + return (v.delta, v.value) else: def sortkey((_k, v)): # sort by overall value - return v[0] + return v.value tavg = 0 for key, values in sorted(stats.items(), key=sortkey, reverse=True): if row >= self.screen.getmaxyx()[0] - 1: break - if not values[0] and not values[1]: + if not values.value and not values.delta: break - if values[0] is not None: - cur = int(round(values[1] / sleeptime)) if values[1] else '' + if values.value is not None: + cur = int(round(values.delta / sleeptime)) if values.delta else '' if self._display_guests: key = self.get_gname_from_pid(key) self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % - (key, values[0], values[0] * 100 / total, + (key, values.value, values.value * 100 / total, cur)) if cur is not '' and key.find('(') is -1: tavg += cur @@ -1375,7 +1378,7 @@ def batch(stats): s = stats.get() for key in sorted(s.keys()): values = s[key] - print('%-42s%10d%10d' % (key, values[0], values[1])) + print('%-42s%10d%10d' % (key, values.value, values.delta)) except KeyboardInterrupt: pass @@ -1392,7 +1395,7 @@ def log(stats): def statline(): s = stats.get() for k in keys: - print(' %9d' % s[k][1], end=' ') + print(' %9d' % s[k].delta, end=' ') print() line = 0 banner_repeat = 20 -- cgit v1.2.3 From 0eb578009a1d530a11846d7c4733a5db04730884 Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 9 Jan 2018 13:27:03 +0100 Subject: tools/kvm_stat: use a more pythonic way to iterate over dictionaries If it's clear that the values of a dictionary will be used then use the '.items()' method. Signed-off-by: Marc Hartmayer Tested-by: Stefan Raspl [Include fix for logging mode by Stefan Raspl] Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 2b7e83a5f7b8..f0da954a856c 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1084,9 +1084,10 @@ class Tui(object): self.screen.clrtobot() stats = self.stats.get(self._display_guests) total = 0. - for key in stats.keys(): + for key, values in stats.items(): if key.find('(') is -1: - total += stats[key].value + total += values.value + if self._sorting == SORT_DEFAULT: def sortkey((_k, v)): # sort by (delta value, overall value) @@ -1376,8 +1377,7 @@ def batch(stats): s = stats.get() time.sleep(1) s = stats.get() - for key in sorted(s.keys()): - values = s[key] + for key, values in sorted(s.items()): print('%-42s%10d%10d' % (key, values.value, values.delta)) except KeyboardInterrupt: pass @@ -1388,14 +1388,14 @@ def log(stats): keys = sorted(stats.get().keys()) def banner(): - for k in keys: - print(k, end=' ') + for key in keys: + print(key, end=' ') print() def statline(): s = stats.get() - for k in keys: - print(' %9d' % s[k].delta, end=' ') + for key in keys: + print(' %9d' % s[key].delta, end=' ') print() line = 0 banner_repeat = 20 -- cgit v1.2.3 From 369d5a85bb782ecf63c5bae9686c7e6104eea991 Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 9 Jan 2018 13:27:04 +0100 Subject: tools/kvm_stat: avoid 'is' for equality checks Use '==' for equality checks and 'is' when comparing identities. An example where '==' and 'is' behave differently: >>> a = 4242 >>> a == 4242 True >>> a is 4242 False Signed-off-by: Marc Hartmayer Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index f0da954a856c..e3f0becb6632 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1085,7 +1085,7 @@ class Tui(object): stats = self.stats.get(self._display_guests) total = 0. for key, values in stats.items(): - if key.find('(') is -1: + if key.find('(') == -1: total += values.value if self._sorting == SORT_DEFAULT: @@ -1110,7 +1110,7 @@ class Tui(object): self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key, values.value, values.value * 100 / total, cur)) - if cur is not '' and key.find('(') is -1: + if cur != '' and key.find('(') == -1: tavg += cur row += 1 if row == 3: -- cgit v1.2.3 From 3df33a0f34a3883b6696bff8cc8fcda3c7444a62 Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Mon, 5 Feb 2018 13:59:57 +0100 Subject: tools/kvm_stat: fix crash when filtering out all non-child trace events When we apply a filter that will only leave child trace events, we receive a ZeroDivisionError when calculating the percentages. In that case, provide percentages based on child events only. To reproduce, run 'kvm_stat -f .*[\(].*'. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index e3f0becb6632..4e0f282c5289 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1084,9 +1084,15 @@ class Tui(object): self.screen.clrtobot() stats = self.stats.get(self._display_guests) total = 0. + ctotal = 0. for key, values in stats.items(): if key.find('(') == -1: total += values.value + else: + ctotal += values.value + if total == 0.: + # we don't have any fields, or all non-child events are filtered + total = ctotal if self._sorting == SORT_DEFAULT: def sortkey((_k, v)): -- cgit v1.2.3 From 1cd8bfb1ed9962be6d80d5020508922aa93653ac Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Mon, 5 Feb 2018 13:59:58 +0100 Subject: tools/kvm_stat: print error on invalid regex Entering an invalid regular expression did not produce any indication of an error so far. To reproduce, press 'f' and enter 'foo(' (with an unescaped bracket). Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 4e0f282c5289..08f842238c32 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1176,6 +1176,7 @@ class Tui(object): Asks for a valid regex and sets the fields filter accordingly. """ + msg = '' while True: self.screen.erase() self.screen.addstr(0, 0, @@ -1184,6 +1185,7 @@ class Tui(object): self.screen.addstr(2, 0, "Current regex: {0}" .format(self.stats.fields_filter)) + self.screen.addstr(5, 0, msg) self.screen.addstr(3, 0, "New regex: ") curses.echo() regex = self.screen.getstr().decode(ENCODING) @@ -1198,6 +1200,7 @@ class Tui(object): self.refresh_header() return except re.error: + msg = '"' + regex + '": Not a valid regular expression' continue def show_vm_selection_by_pid(self): -- cgit v1.2.3 From 1fd6a708c8438403dee17eb411cf81ffba13cf43 Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:24 +0100 Subject: tools/kvm_stat: fix debugfs handling Te checks for debugfs assumed that debugfs is always mounted at /sys/kernel/debug - which is likely, but not guaranteed. This is addressed by checking /proc/mounts for the actual location. Furthermore, when debugfs was mounted, but the kvm module not loaded, a misleading error pointing towards debugfs not present was given. To reproduce, (a) run kvm_stat with debugfs mounted at a place different from /sys/kernel/debug (b) run kvm_stat with debugfs mounted but kvm module not loaded Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 08f842238c32..c5c8e9295b91 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -331,9 +331,6 @@ class perf_event_attr(ctypes.Structure): PERF_TYPE_TRACEPOINT = 2 PERF_FORMAT_GROUP = 1 << 3 -PATH_DEBUGFS_TRACING = '/sys/kernel/debug/tracing' -PATH_DEBUGFS_KVM = '/sys/kernel/debug/kvm' - class Group(object): """Represents a perf event group.""" @@ -1544,17 +1541,6 @@ Press any other key to refresh statistics immediately. def check_access(options): """Exits if the current user can't access all needed directories.""" - if not os.path.exists('/sys/kernel/debug'): - sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.') - sys.exit(1) - - if not os.path.exists(PATH_DEBUGFS_KVM): - sys.stderr.write("Please make sure, that debugfs is mounted and " - "readable by the current user:\n" - "('mount -t debugfs debugfs /sys/kernel/debug')\n" - "Also ensure, that the kvm modules are loaded.\n") - sys.exit(1) - if not os.path.exists(PATH_DEBUGFS_TRACING) and (options.tracepoints or not options.debugfs): sys.stderr.write("Please enable CONFIG_TRACING in your kernel " @@ -1572,7 +1558,33 @@ def check_access(options): return options +def assign_globals(): + global PATH_DEBUGFS_KVM + global PATH_DEBUGFS_TRACING + + debugfs = '' + for line in file('/proc/mounts'): + if line.split(' ')[0] == 'debugfs': + debugfs = line.split(' ')[1] + break + if debugfs == '': + sys.stderr.write("Please make sure that CONFIG_DEBUG_FS is enabled in " + "your kernel, mounted and\nreadable by the current " + "user:\n" + "('mount -t debugfs debugfs /sys/kernel/debug')\n") + sys.exit(1) + + PATH_DEBUGFS_KVM = os.path.join(debugfs, 'kvm') + PATH_DEBUGFS_TRACING = os.path.join(debugfs, 'tracing') + + if not os.path.exists(PATH_DEBUGFS_KVM): + sys.stderr.write("Please make sure that CONFIG_KVM is enabled in " + "your kernel and that the modules are loaded.\n") + sys.exit(1) + + def main(): + assign_globals() options = get_options() options = check_access(options) -- cgit v1.2.3 From c0e8c21eae616ed8703c1b4b01046a1578ee875c Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:26 +0100 Subject: tools/kvm_stat: mark private methods as such Helps quite a bit reading the code when it's obvious when a method is intended for internal use only. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 132 ++++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 66 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index c5c8e9295b91..c09b7428f563 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -373,8 +373,8 @@ class Event(object): self.syscall = self.libc.syscall self.name = name self.fd = None - self.setup_event(group, trace_cpu, trace_pid, trace_point, - trace_filter, trace_set) + self._setup_event(group, trace_cpu, trace_pid, trace_point, + trace_filter, trace_set) def __del__(self): """Closes the event's file descriptor. @@ -387,7 +387,7 @@ class Event(object): if self.fd: os.close(self.fd) - def perf_event_open(self, attr, pid, cpu, group_fd, flags): + def _perf_event_open(self, attr, pid, cpu, group_fd, flags): """Wrapper for the sys_perf_evt_open() syscall. Used to set up performance events, returns a file descriptor or -1 @@ -406,7 +406,7 @@ class Event(object): ctypes.c_int(pid), ctypes.c_int(cpu), ctypes.c_int(group_fd), ctypes.c_long(flags)) - def setup_event_attribute(self, trace_set, trace_point): + def _setup_event_attribute(self, trace_set, trace_point): """Returns an initialized ctype perf_event_attr struct.""" id_path = os.path.join(PATH_DEBUGFS_TRACING, 'events', trace_set, @@ -416,8 +416,8 @@ class Event(object): event_attr.config = int(open(id_path).read()) return event_attr - def setup_event(self, group, trace_cpu, trace_pid, trace_point, - trace_filter, trace_set): + def _setup_event(self, group, trace_cpu, trace_pid, trace_point, + trace_filter, trace_set): """Sets up the perf event in Linux. Issues the syscall to register the event in the kernel and @@ -425,7 +425,7 @@ class Event(object): """ - event_attr = self.setup_event_attribute(trace_set, trace_point) + event_attr = self._setup_event_attribute(trace_set, trace_point) # First event will be group leader. group_leader = -1 @@ -434,8 +434,8 @@ class Event(object): if group.events: group_leader = group.events[0].fd - fd = self.perf_event_open(event_attr, trace_pid, - trace_cpu, group_leader, 0) + fd = self._perf_event_open(event_attr, trace_pid, + trace_cpu, group_leader, 0) if fd == -1: err = ctypes.get_errno() raise OSError(err, os.strerror(err), @@ -497,12 +497,12 @@ class TracepointProvider(Provider): """ def __init__(self, pid, fields_filter): self.group_leaders = [] - self.filters = self.get_filters() + self.filters = self._get_filters() self.update_fields(fields_filter) self.pid = pid @staticmethod - def get_filters(): + def _get_filters(): """Returns a dict of trace events, their filter ids and the values that can be filtered. @@ -518,7 +518,7 @@ class TracepointProvider(Provider): filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons) return filters - def get_available_fields(self): + def _get_available_fields(self): """Returns a list of available event's of format 'event name(filter name)'. @@ -546,11 +546,11 @@ class TracepointProvider(Provider): def update_fields(self, fields_filter): """Refresh fields, applying fields_filter""" - self.fields = [field for field in self.get_available_fields() + self.fields = [field for field in self._get_available_fields() if self.is_field_wanted(fields_filter, field)] @staticmethod - def get_online_cpus(): + def _get_online_cpus(): """Returns a list of cpu id integers.""" def parse_int_list(list_string): """Returns an int list from a string of comma separated integers and @@ -572,17 +572,17 @@ class TracepointProvider(Provider): cpu_string = cpu_list.readline() return parse_int_list(cpu_string) - def setup_traces(self): + def _setup_traces(self): """Creates all event and group objects needed to be able to retrieve data.""" - fields = self.get_available_fields() + fields = self._get_available_fields() if self._pid > 0: # Fetch list of all threads of the monitored pid, as qemu # starts a thread for each vcpu. path = os.path.join('/proc', str(self._pid), 'task') groupids = self.walkdir(path)[1] else: - groupids = self.get_online_cpus() + groupids = self._get_online_cpus() # The constant is needed as a buffer for python libs, std # streams and other files that the script opens. @@ -660,7 +660,7 @@ class TracepointProvider(Provider): # The garbage collector will get rid of all Event/Group # objects and open files after removing the references. self.group_leaders = [] - self.setup_traces() + self._setup_traces() self.fields = self._fields def read(self, by_guest=0): @@ -689,9 +689,9 @@ class DebugfsProvider(Provider): self.paths = [] self.pid = pid if include_past: - self.restore() + self._restore() - def get_available_fields(self): + def _get_available_fields(self): """"Returns a list of available fields. The fields are all available KVM debugfs files @@ -701,7 +701,7 @@ class DebugfsProvider(Provider): def update_fields(self, fields_filter): """Refresh fields, applying fields_filter""" - self._fields = [field for field in self.get_available_fields() + self._fields = [field for field in self._get_available_fields() if self.is_field_wanted(fields_filter, field)] @property @@ -755,7 +755,7 @@ class DebugfsProvider(Provider): paths.append(dir) for path in paths: for field in self._fields: - value = self.read_field(field, path) + value = self._read_field(field, path) key = path + field if reset == 1: self._baseline[key] = value @@ -776,7 +776,7 @@ class DebugfsProvider(Provider): return results - def read_field(self, field, path): + def _read_field(self, field, path): """Returns the value of a single field from a specific VM.""" try: return int(open(os.path.join(PATH_DEBUGFS_KVM, @@ -791,7 +791,7 @@ class DebugfsProvider(Provider): self._baseline = {} self.read(1) - def restore(self): + def _restore(self): """Reset field counters""" self._baseline = {} self.read(2) @@ -808,13 +808,12 @@ class Stats(object): """ def __init__(self, options): - self.providers = self.get_providers(options) + self.providers = self._get_providers(options) self._pid_filter = options.pid self._fields_filter = options.fields self.values = {} - @staticmethod - def get_providers(options): + def _get_providers(self, options): """Returns a list of data providers depending on the passed options.""" providers = [] @@ -826,7 +825,7 @@ class Stats(object): return providers - def update_provider_filters(self): + def _update_provider_filters(self): """Propagates fields filters to providers.""" # As we reset the counters when updating the fields we can # also clear the cache of old values. @@ -847,7 +846,7 @@ class Stats(object): def fields_filter(self, fields_filter): if fields_filter != self._fields_filter: self._fields_filter = fields_filter - self.update_provider_filters() + self._update_provider_filters() @property def pid_filter(self): @@ -969,7 +968,7 @@ class Tui(object): return res - def print_all_gnames(self, row): + def _print_all_gnames(self, row): """Print a list of all running guests along with their pids.""" self.screen.addstr(row, 2, '%8s %-60s' % ('Pid', 'Guest Name (fuzzy list, might be ' @@ -1032,7 +1031,7 @@ class Tui(object): return name - def update_drilldown(self): + def _update_drilldown(self): """Sets or removes a filter that only allows fields without braces.""" if not self.stats.fields_filter: self.stats.fields_filter = DEFAULT_REGEX @@ -1040,11 +1039,11 @@ class Tui(object): elif self.stats.fields_filter == DEFAULT_REGEX: self.stats.fields_filter = None - def update_pid(self, pid): + def _update_pid(self, pid): """Propagates pid selection to stats object.""" self.stats.pid_filter = pid - def refresh_header(self, pid=None): + def _refresh_header(self, pid=None): """Refreshes the header.""" if pid is None: pid = self.stats.pid_filter @@ -1075,7 +1074,7 @@ class Tui(object): self.screen.addstr(4, 1, 'Collecting data...') self.screen.refresh() - def refresh_body(self, sleeptime): + def _refresh_body(self, sleeptime): row = 3 self.screen.move(row, 0) self.screen.clrtobot() @@ -1124,7 +1123,7 @@ class Tui(object): curses.A_BOLD) self.screen.refresh() - def show_msg(self, text): + def _show_msg(self, text): """Display message centered text and exit on key press""" hint = 'Press any key to continue' curses.cbreak() @@ -1139,7 +1138,7 @@ class Tui(object): curses.A_STANDOUT) self.screen.getkey() - def show_help_interactive(self): + def _show_help_interactive(self): """Display help with list of interactive commands""" msg = (' b toggle events by guests (debugfs only, honors' ' filters)', @@ -1165,9 +1164,9 @@ class Tui(object): self.screen.addstr(row, 0, line) row += 1 self.screen.getkey() - self.refresh_header() + self._refresh_header() - def show_filter_selection(self): + def _show_filter_selection(self): """Draws filter selection mask. Asks for a valid regex and sets the fields filter accordingly. @@ -1189,18 +1188,18 @@ class Tui(object): curses.noecho() if len(regex) == 0: self.stats.fields_filter = DEFAULT_REGEX - self.refresh_header() + self._refresh_header() return try: re.compile(regex) self.stats.fields_filter = regex - self.refresh_header() + self._refresh_header() return except re.error: msg = '"' + regex + '": Not a valid regular expression' continue - def show_vm_selection_by_pid(self): + def _show_vm_selection_by_pid(self): """Draws PID selection mask. Asks for a pid until a valid pid or 0 has been entered. @@ -1216,7 +1215,7 @@ class Tui(object): 'This might limit the shown data to the trace ' 'statistics.') self.screen.addstr(5, 0, msg) - self.print_all_gnames(7) + self._print_all_gnames(7) curses.echo() self.screen.addstr(3, 0, "Pid [0 or pid]: ") @@ -1232,13 +1231,13 @@ class Tui(object): continue else: pid = 0 - self.refresh_header(pid) - self.update_pid(pid) + self._refresh_header(pid) + self._update_pid(pid) break except ValueError: msg = '"' + str(pid) + '": Not a valid pid' - def show_set_update_interval(self): + def _show_set_update_interval(self): """Draws update interval selection mask.""" msg = '' while True: @@ -1268,9 +1267,9 @@ class Tui(object): except ValueError: msg = '"' + str(val) + '": Invalid value' - self.refresh_header() + self._refresh_header() - def show_vm_selection_by_guest_name(self): + def _show_vm_selection_by_guest_name(self): """Draws guest selection mask. Asks for a guest name until a valid guest name or '' is entered. @@ -1286,15 +1285,15 @@ class Tui(object): 'This might limit the shown data to the trace ' 'statistics.') self.screen.addstr(5, 0, msg) - self.print_all_gnames(7) + self._print_all_gnames(7) curses.echo() self.screen.addstr(3, 0, "Guest [ENTER or guest]: ") gname = self.screen.getstr().decode(ENCODING) curses.noecho() if not gname: - self.refresh_header(0) - self.update_pid(0) + self._refresh_header(0) + self._update_pid(0) break else: pids = [] @@ -1311,17 +1310,17 @@ class Tui(object): msg = '"' + gname + '": Multiple matches found, use pid ' \ 'filter instead' continue - self.refresh_header(pids[0]) - self.update_pid(pids[0]) + self._refresh_header(pids[0]) + self._update_pid(pids[0]) break def show_stats(self): """Refreshes the screen and processes user input.""" sleeptime = self._delay_initial - self.refresh_header() + self._refresh_header() start = 0.0 # result based on init value never appears on screen while True: - self.refresh_body(time.time() - start) + self._refresh_body(time.time() - start) curses.halfdelay(int(sleeptime * 10)) start = time.time() sleeptime = self._delay_regular @@ -1330,32 +1329,33 @@ class Tui(object): if char == 'b': self._display_guests = not self._display_guests if self.stats.toggle_display_guests(self._display_guests): - self.show_msg(['Command not available with tracepoints' - ' enabled', 'Restart with debugfs only ' - '(see option \'-d\') and try again!']) + self._show_msg(['Command not available with ' + 'tracepoints enabled', 'Restart with ' + 'debugfs only (see option \'-d\') and ' + 'try again!']) self._display_guests = not self._display_guests - self.refresh_header() + self._refresh_header() if char == 'c': self.stats.fields_filter = DEFAULT_REGEX - self.refresh_header(0) - self.update_pid(0) + self._refresh_header(0) + self._update_pid(0) if char == 'f': curses.curs_set(1) - self.show_filter_selection() + self._show_filter_selection() curses.curs_set(0) sleeptime = self._delay_initial if char == 'g': curses.curs_set(1) - self.show_vm_selection_by_guest_name() + self._show_vm_selection_by_guest_name() curses.curs_set(0) sleeptime = self._delay_initial if char == 'h': - self.show_help_interactive() + self._show_help_interactive() if char == 'o': self._sorting = not self._sorting if char == 'p': curses.curs_set(1) - self.show_vm_selection_by_pid() + self._show_vm_selection_by_pid() curses.curs_set(0) sleeptime = self._delay_initial if char == 'q': @@ -1364,11 +1364,11 @@ class Tui(object): self.stats.reset() if char == 's': curses.curs_set(1) - self.show_set_update_interval() + self._show_set_update_interval() curses.curs_set(0) sleeptime = self._delay_initial if char == 'x': - self.update_drilldown() + self._update_drilldown() # prevents display of current values on next refresh self.stats.get(self._display_guests) except KeyboardInterrupt: -- cgit v1.2.3 From 516f1190a1e0cce12128a6446e6438745c8de62a Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:27 +0100 Subject: tools/kvm_stat: eliminate extra guest/pid selection dialog We can do with a single dialog that takes both, pids and guest names. Note that we keep both interactive commands, 'p' and 'g' for now, to avoid confusion among users used to a specific key. While at it, we improve on some minor glitches regarding curses usage, e.g. cursor still visible when not supposed to be. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 110 ++++++++++++++-------------------------- tools/kvm/kvm_stat/kvm_stat.txt | 4 +- 2 files changed, 39 insertions(+), 75 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index c09b7428f563..0d5776785d27 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1041,6 +1041,8 @@ class Tui(object): def _update_pid(self, pid): """Propagates pid selection to stats object.""" + self.screen.addstr(4, 1, 'Updating pid filter...') + self.screen.refresh() self.stats.pid_filter = pid def _refresh_header(self, pid=None): @@ -1144,10 +1146,10 @@ class Tui(object): ' filters)', ' c clear filter', ' f filter by regular expression', - ' g filter by guest name', + ' g filter by guest name/PID', ' h display interactive commands reference', ' o toggle sorting order (Total vs CurAvg/s)', - ' p filter by PID', + ' p filter by guest name/PID', ' q quit', ' r reset stats', ' s set update interval', @@ -1199,44 +1201,6 @@ class Tui(object): msg = '"' + regex + '": Not a valid regular expression' continue - def _show_vm_selection_by_pid(self): - """Draws PID selection mask. - - Asks for a pid until a valid pid or 0 has been entered. - - """ - msg = '' - while True: - self.screen.erase() - self.screen.addstr(0, 0, - 'Show statistics for specific pid.', - curses.A_BOLD) - self.screen.addstr(1, 0, - 'This might limit the shown data to the trace ' - 'statistics.') - self.screen.addstr(5, 0, msg) - self._print_all_gnames(7) - - curses.echo() - self.screen.addstr(3, 0, "Pid [0 or pid]: ") - pid = self.screen.getstr().decode(ENCODING) - curses.noecho() - - try: - if len(pid) > 0: - pid = int(pid) - if pid != 0 and not os.path.isdir(os.path.join('/proc/', - str(pid))): - msg = '"' + str(pid) + '": Not a running process' - continue - else: - pid = 0 - self._refresh_header(pid) - self._update_pid(pid) - break - except ValueError: - msg = '"' + str(pid) + '": Not a valid pid' - def _show_set_update_interval(self): """Draws update interval selection mask.""" msg = '' @@ -1269,17 +1233,17 @@ class Tui(object): msg = '"' + str(val) + '": Invalid value' self._refresh_header() - def _show_vm_selection_by_guest_name(self): + def _show_vm_selection_by_guest(self): """Draws guest selection mask. - Asks for a guest name until a valid guest name or '' is entered. + Asks for a guest name or pid until a valid guest name or '' is entered. """ msg = '' while True: self.screen.erase() self.screen.addstr(0, 0, - 'Show statistics for specific guest.', + 'Show statistics for specific guest or pid.', curses.A_BOLD) self.screen.addstr(1, 0, 'This might limit the shown data to the trace ' @@ -1287,32 +1251,39 @@ class Tui(object): self.screen.addstr(5, 0, msg) self._print_all_gnames(7) curses.echo() - self.screen.addstr(3, 0, "Guest [ENTER or guest]: ") - gname = self.screen.getstr().decode(ENCODING) + curses.curs_set(1) + self.screen.addstr(3, 0, "Guest or pid [ENTER exits]: ") + guest = self.screen.getstr().decode(ENCODING) curses.noecho() - if not gname: - self._refresh_header(0) - self._update_pid(0) + pid = 0 + if not guest or guest == '0': break - else: - pids = [] - try: - pids = self.get_pid_from_gname(gname) - except: - msg = '"' + gname + '": Internal error while searching, ' \ - 'use pid filter instead' - continue - if len(pids) == 0: - msg = '"' + gname + '": Not an active guest' + if guest.isdigit(): + if not os.path.isdir(os.path.join('/proc/', guest)): + msg = '"' + guest + '": Not a running process' continue - if len(pids) > 1: - msg = '"' + gname + '": Multiple matches found, use pid ' \ - 'filter instead' - continue - self._refresh_header(pids[0]) - self._update_pid(pids[0]) + pid = int(guest) break + pids = [] + try: + pids = self.get_pid_from_gname(guest) + except: + msg = '"' + guest + '": Internal error while searching, ' \ + 'use pid filter instead' + continue + if len(pids) == 0: + msg = '"' + guest + '": Not an active guest' + continue + if len(pids) > 1: + msg = '"' + guest + '": Multiple matches found, use pid ' \ + 'filter instead' + continue + pid = pids[0] + break + curses.curs_set(0) + self._refresh_header(pid) + self._update_pid(pid) def show_stats(self): """Refreshes the screen and processes user input.""" @@ -1344,20 +1315,13 @@ class Tui(object): self._show_filter_selection() curses.curs_set(0) sleeptime = self._delay_initial - if char == 'g': - curses.curs_set(1) - self._show_vm_selection_by_guest_name() - curses.curs_set(0) + if char == 'g' or char == 'p': + self._show_vm_selection_by_guest() sleeptime = self._delay_initial if char == 'h': self._show_help_interactive() if char == 'o': self._sorting = not self._sorting - if char == 'p': - curses.curs_set(1) - self._show_vm_selection_by_pid() - curses.curs_set(0) - sleeptime = self._delay_initial if char == 'q': break if char == 'r': diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt index b5b3810c9e94..0811d860fe75 100644 --- a/tools/kvm/kvm_stat/kvm_stat.txt +++ b/tools/kvm/kvm_stat/kvm_stat.txt @@ -35,13 +35,13 @@ INTERACTIVE COMMANDS *f*:: filter by regular expression -*g*:: filter by guest name +*g*:: filter by guest name/PID *h*:: display interactive commands reference *o*:: toggle sorting order (Total vs CurAvg/s) -*p*:: filter by PID +*p*:: filter by guest name/PID *q*:: quit -- cgit v1.2.3 From 18e8f4100ef14f924514fbd91eb67bd5fa5396b7 Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:28 +0100 Subject: tools/kvm_stat: separate drilldown and fields filtering Drilldown (i.e. toggle display of child trace events) was implemented by overriding the fields filter. This resulted in inconsistencies: E.g. when drilldown was not active, adding a filter that also matches child trace events would not only filter fields according to the filter, but also add in the child trace events matching the filter. E.g. on x86, setting 'kvm_userspace_exit' as the fields filter after startup would result in display of kvm_userspace_exit(DCR), although that wasn't previously present - not exactly what one would expect from a filter. This patch addresses the issue by keeping drilldown and fields filter separate. While at it, we also fix a PEP8 issue by adding a blank line at one place (since we're in the area...). We implement this by adding a framework that also allows to define a taxonomy among the debugfs events to identify child trace events. I.e. drilldown using 'x' can now also work with debugfs. A respective parent- child relationship is only known for S390 at the moment, but could be added adjusting other platforms' ARCH.dbg_is_child() methods accordingly. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 143 +++++++++++++++++++++++++++++++------------- 1 file changed, 100 insertions(+), 43 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 0d5776785d27..6b6630ee6daf 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -228,6 +228,7 @@ IOCTL_NUMBERS = { } ENCODING = locale.getpreferredencoding(False) +TRACE_FILTER = re.compile(r'^[^\(]*$') class Arch(object): @@ -260,6 +261,11 @@ class Arch(object): return ArchX86(SVM_EXIT_REASONS) return + def tracepoint_is_child(self, field): + if (TRACE_FILTER.match(field)): + return None + return field.split('(', 1)[0] + class ArchX86(Arch): def __init__(self, exit_reasons): @@ -267,6 +273,10 @@ class ArchX86(Arch): self.ioctl_numbers = IOCTL_NUMBERS self.exit_reasons = exit_reasons + def debugfs_is_child(self, field): + """ Returns name of parent if 'field' is a child, None otherwise """ + return None + class ArchPPC(Arch): def __init__(self): @@ -282,6 +292,10 @@ class ArchPPC(Arch): self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16 self.exit_reasons = {} + def debugfs_is_child(self, field): + """ Returns name of parent if 'field' is a child, None otherwise """ + return None + class ArchA64(Arch): def __init__(self): @@ -289,6 +303,10 @@ class ArchA64(Arch): self.ioctl_numbers = IOCTL_NUMBERS self.exit_reasons = AARCH64_EXIT_REASONS + def debugfs_is_child(self, field): + """ Returns name of parent if 'field' is a child, None otherwise """ + return None + class ArchS390(Arch): def __init__(self): @@ -296,6 +314,12 @@ class ArchS390(Arch): self.ioctl_numbers = IOCTL_NUMBERS self.exit_reasons = None + def debugfs_is_child(self, field): + """ Returns name of parent if 'field' is a child, None otherwise """ + if field.startswith('instruction_'): + return 'exit_instruction' + + ARCH = Arch.get_arch() @@ -472,6 +496,10 @@ class Event(object): class Provider(object): """Encapsulates functionalities used by all providers.""" + def __init__(self, pid): + self.child_events = False + self.pid = pid + @staticmethod def is_field_wanted(fields_filter, field): """Indicate whether field is valid according to fields_filter.""" @@ -499,7 +527,7 @@ class TracepointProvider(Provider): self.group_leaders = [] self.filters = self._get_filters() self.update_fields(fields_filter) - self.pid = pid + super(TracepointProvider, self).__init__(pid) @staticmethod def _get_filters(): @@ -519,7 +547,7 @@ class TracepointProvider(Provider): return filters def _get_available_fields(self): - """Returns a list of available event's of format 'event name(filter + """Returns a list of available events of format 'event name(filter name)'. All available events have directories under @@ -547,7 +575,8 @@ class TracepointProvider(Provider): def update_fields(self, fields_filter): """Refresh fields, applying fields_filter""" self.fields = [field for field in self._get_available_fields() - if self.is_field_wanted(fields_filter, field)] + if self.is_field_wanted(fields_filter, field) or + ARCH.tracepoint_is_child(field)] @staticmethod def _get_online_cpus(): @@ -668,8 +697,12 @@ class TracepointProvider(Provider): ret = defaultdict(int) for group in self.group_leaders: for name, val in group.read().items(): - if name in self._fields: - ret[name] += val + if name not in self._fields: + continue + parent = ARCH.tracepoint_is_child(name) + if parent: + name += ' ' + parent + ret[name] += val return ret def reset(self): @@ -687,7 +720,7 @@ class DebugfsProvider(Provider): self._baseline = {} self.do_read = True self.paths = [] - self.pid = pid + super(DebugfsProvider, self).__init__(pid) if include_past: self._restore() @@ -702,7 +735,8 @@ class DebugfsProvider(Provider): def update_fields(self, fields_filter): """Refresh fields, applying fields_filter""" self._fields = [field for field in self._get_available_fields() - if self.is_field_wanted(fields_filter, field)] + if self.is_field_wanted(fields_filter, field) or + ARCH.debugfs_is_child(field)] @property def fields(self): @@ -763,14 +797,15 @@ class DebugfsProvider(Provider): self._baseline[key] = 0 if self._baseline.get(key, -1) == -1: self._baseline[key] = value - increment = (results.get(field, 0) + value - - self._baseline.get(key, 0)) - if by_guest: - pid = key.split('-')[0] - if pid in results: - results[pid] += increment - else: - results[pid] = increment + parent = ARCH.debugfs_is_child(field) + if parent: + field = field + ' ' + parent + else: + if by_guest: + field = key.split('-')[0] # set 'field' to 'pid' + increment = value - self._baseline.get(key, 0) + if field in results: + results[field] += increment else: results[field] = increment @@ -812,6 +847,7 @@ class Stats(object): self._pid_filter = options.pid self._fields_filter = options.fields self.values = {} + self._child_events = False def _get_providers(self, options): """Returns a list of data providers depending on the passed options.""" @@ -860,12 +896,29 @@ class Stats(object): for provider in self.providers: provider.pid = self._pid_filter + @property + def child_events(self): + return self._child_events + + @child_events.setter + def child_events(self, val): + self._child_events = val + for provider in self.providers: + provider.child_events = val + def get(self, by_guest=0): """Returns a dict with field -> (value, delta to last value) of all - provider data.""" + provider data. + Key formats: + * plain: 'key' is event name + * child-parent: 'key' is in format ' ' + * pid: 'key' is the pid of the guest, and the record contains the + aggregated event data + These formats are generated by the providers, and handled in class TUI. + """ for provider in self.providers: new = provider.read(by_guest=by_guest) - for key in new if by_guest else provider.fields: + for key in new: oldval = self.values.get(key, EventStat(0, 0)).value newval = new.get(key, 0) newdelta = newval - oldval @@ -898,10 +951,10 @@ class Stats(object): self.get(to_pid) return 0 + DELAY_DEFAULT = 3.0 MAX_GUEST_NAME_LEN = 48 MAX_REGEX_LEN = 44 -DEFAULT_REGEX = r'^[^\(]*$' SORT_DEFAULT = 0 @@ -1031,14 +1084,6 @@ class Tui(object): return name - def _update_drilldown(self): - """Sets or removes a filter that only allows fields without braces.""" - if not self.stats.fields_filter: - self.stats.fields_filter = DEFAULT_REGEX - - elif self.stats.fields_filter == DEFAULT_REGEX: - self.stats.fields_filter = None - def _update_pid(self, pid): """Propagates pid selection to stats object.""" self.screen.addstr(4, 1, 'Updating pid filter...') @@ -1060,8 +1105,7 @@ class Tui(object): .format(pid, gname), curses.A_BOLD) else: self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD) - if self.stats.fields_filter and self.stats.fields_filter \ - != DEFAULT_REGEX: + if self.stats.fields_filter: regex = self.stats.fields_filter if len(regex) > MAX_REGEX_LEN: regex = regex[:MAX_REGEX_LEN] + '...' @@ -1077,6 +1121,9 @@ class Tui(object): self.screen.refresh() def _refresh_body(self, sleeptime): + def is_child_field(field): + return field.find('(') != -1 + row = 3 self.screen.move(row, 0) self.screen.clrtobot() @@ -1084,7 +1131,11 @@ class Tui(object): total = 0. ctotal = 0. for key, values in stats.items(): - if key.find('(') == -1: + if self._display_guests: + if self.get_gname_from_pid(key): + total += values.value + continue + if not key.find(' ') != -1: total += values.value else: ctotal += values.value @@ -1101,19 +1152,26 @@ class Tui(object): # sort by overall value return v.value + sorted_items = sorted(stats.items(), key=sortkey, reverse=True) + + # print events tavg = 0 - for key, values in sorted(stats.items(), key=sortkey, reverse=True): + for key, values in sorted_items: if row >= self.screen.getmaxyx()[0] - 1: break - if not values.value and not values.delta: - break + if values == (0, 0): + continue + if not self.stats.child_events and key.find(' ') != -1: + continue if values.value is not None: cur = int(round(values.delta / sleeptime)) if values.delta else '' if self._display_guests: key = self.get_gname_from_pid(key) - self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % - (key, values.value, values.value * 100 / total, - cur)) + if not key: + continue + self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key + .split(' ')[0], values.value, + values.value * 100 / total, cur)) if cur != '' and key.find('(') == -1: tavg += cur row += 1 @@ -1189,7 +1247,7 @@ class Tui(object): regex = self.screen.getstr().decode(ENCODING) curses.noecho() if len(regex) == 0: - self.stats.fields_filter = DEFAULT_REGEX + self.stats.fields_filter = '' self._refresh_header() return try: @@ -1307,7 +1365,7 @@ class Tui(object): self._display_guests = not self._display_guests self._refresh_header() if char == 'c': - self.stats.fields_filter = DEFAULT_REGEX + self.stats.fields_filter = '' self._refresh_header(0) self._update_pid(0) if char == 'f': @@ -1332,9 +1390,7 @@ class Tui(object): curses.curs_set(0) sleeptime = self._delay_initial if char == 'x': - self._update_drilldown() - # prevents display of current values on next refresh - self.stats.get(self._display_guests) + self.stats.child_events = not self.stats.child_events except KeyboardInterrupt: break except curses.error: @@ -1348,7 +1404,8 @@ def batch(stats): time.sleep(1) s = stats.get() for key, values in sorted(s.items()): - print('%-42s%10d%10d' % (key, values.value, values.delta)) + print('%-42s%10d%10d' % (key.split(' ')[0], values.value, + values.delta)) except KeyboardInterrupt: pass @@ -1359,7 +1416,7 @@ def log(stats): def banner(): for key in keys: - print(key, end=' ') + print(key.split(' ')[0], end=' ') print() def statline(): @@ -1470,7 +1527,7 @@ Press any other key to refresh statistics immediately. ) optparser.add_option('-f', '--fields', action='store', - default=DEFAULT_REGEX, + default='', dest='fields', help='''fields to display (regex) "-f help" for a list of available events''', -- cgit v1.2.3 From df72ecfc790fa01de1c41f836ff51d12f9c40318 Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:29 +0100 Subject: tools/kvm_stat: group child events indented after parent We keep the current logic that sorts all events (parent and child), but re-shuffle the events afterwards, grouping the children after the respective parent. Note that the percentage column for child events gives the percentage of the parent's total. Since we rework the logic anyway, we modify the total average calculation to use the raw numbers instead of the (rounded) averages. Note that this can result in differing numbers (between total average and the sum of the individual averages) due to rounding errors. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 89 ++++++++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 30 deletions(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 6b6630ee6daf..862c997932e2 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1124,6 +1124,45 @@ class Tui(object): def is_child_field(field): return field.find('(') != -1 + def insert_child(sorted_items, child, values, parent): + num = len(sorted_items) + for i in range(0, num): + # only add child if parent is present + if parent.startswith(sorted_items[i][0]): + sorted_items.insert(i + 1, (' ' + child, values)) + + def get_sorted_events(self, stats): + """ separate parent and child events """ + if self._sorting == SORT_DEFAULT: + def sortkey((_k, v)): + # sort by (delta value, overall value) + return (v.delta, v.value) + else: + def sortkey((_k, v)): + # sort by overall value + return v.value + + childs = [] + sorted_items = [] + # we can't rule out child events to appear prior to parents even + # when sorted - separate out all children first, and add in later + for key, values in sorted(stats.items(), key=sortkey, + reverse=True): + if values == (0, 0): + continue + if key.find(' ') != -1: + if not self.stats.child_events: + continue + childs.insert(0, (key, values)) + else: + sorted_items.append((key, values)) + if self.stats.child_events: + for key, values in childs: + (child, parent) = key.split(' ') + insert_child(sorted_items, child, values, parent) + + return sorted_items + row = 3 self.screen.move(row, 0) self.screen.clrtobot() @@ -1143,44 +1182,34 @@ class Tui(object): # we don't have any fields, or all non-child events are filtered total = ctotal - if self._sorting == SORT_DEFAULT: - def sortkey((_k, v)): - # sort by (delta value, overall value) - return (v.delta, v.value) - else: - def sortkey((_k, v)): - # sort by overall value - return v.value - - sorted_items = sorted(stats.items(), key=sortkey, reverse=True) - # print events tavg = 0 - for key, values in sorted_items: - if row >= self.screen.getmaxyx()[0] - 1: + tcur = 0 + for key, values in get_sorted_events(self, stats): + if row >= self.screen.getmaxyx()[0] - 1 or values == (0, 0): break - if values == (0, 0): - continue - if not self.stats.child_events and key.find(' ') != -1: - continue - if values.value is not None: - cur = int(round(values.delta / sleeptime)) if values.delta else '' - if self._display_guests: - key = self.get_gname_from_pid(key) - if not key: - continue - self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key - .split(' ')[0], values.value, - values.value * 100 / total, cur)) - if cur != '' and key.find('(') == -1: - tavg += cur + if self._display_guests: + key = self.get_gname_from_pid(key) + if not key: + continue + cur = int(round(values.delta / sleeptime)) if values.delta else '' + if key[0] != ' ': + if values.delta: + tcur += values.delta + ptotal = values.value + ltotal = total + else: + ltotal = ptotal + self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' % (key, + values.value, + values.value * 100 / float(ltotal), cur)) row += 1 if row == 3: self.screen.addstr(4, 1, 'No matching events reported yet') else: + tavg = int(round(tcur / sleeptime)) if tcur > 0 else '' self.screen.addstr(row, 1, '%-40s %10d %8s' % - ('Total', total, tavg if tavg else ''), - curses.A_BOLD) + ('Total', total, tavg), curses.A_BOLD) self.screen.refresh() def _show_msg(self, text): -- cgit v1.2.3 From 6789af030a462708f937137e05eb12ea009fb348 Mon Sep 17 00:00:00 2001 From: Stefan Raspl Date: Thu, 22 Feb 2018 12:16:30 +0100 Subject: tools/kvm_stat: print 'Total' line for multiple events only The 'Total' line looks a bit weird when we have a single event only. This can happen e.g. due to filters. Therefore suppress when there's only a single event in the output. Signed-off-by: Stefan Raspl Signed-off-by: Paolo Bonzini --- tools/kvm/kvm_stat/kvm_stat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat index 862c997932e2..5898c22ba310 100755 --- a/tools/kvm/kvm_stat/kvm_stat +++ b/tools/kvm/kvm_stat/kvm_stat @@ -1206,7 +1206,7 @@ class Tui(object): row += 1 if row == 3: self.screen.addstr(4, 1, 'No matching events reported yet') - else: + if row > 4: tavg = int(round(tcur / sleeptime)) if tcur > 0 else '' self.screen.addstr(row, 1, '%-40s %10d %8s' % ('Total', total, tavg), curses.A_BOLD) -- cgit v1.2.3 From 3d4d5d618639c3155cfce57101d619a0935434d2 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Sun, 25 Feb 2018 06:00:11 -0500 Subject: radix tree test suite: Fix build - Add an empty linux/compiler_types.h (now being included by kconfig.h) - Add __GFP_ZERO - Add kzalloc - Test __GFP_DIRECT_RECLAIM instead of __GFP_NOWARN Signed-off-by: Matthew Wilcox --- tools/testing/radix-tree/linux.c | 11 +++++++++-- tools/testing/radix-tree/linux/compiler_types.h | 0 tools/testing/radix-tree/linux/gfp.h | 1 + tools/testing/radix-tree/linux/slab.h | 6 ++++++ 4 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 tools/testing/radix-tree/linux/compiler_types.h (limited to 'tools') diff --git a/tools/testing/radix-tree/linux.c b/tools/testing/radix-tree/linux.c index 6903ccf35595..44a0d1ad4408 100644 --- a/tools/testing/radix-tree/linux.c +++ b/tools/testing/radix-tree/linux.c @@ -29,7 +29,7 @@ void *kmem_cache_alloc(struct kmem_cache *cachep, int flags) { struct radix_tree_node *node; - if (flags & __GFP_NOWARN) + if (!(flags & __GFP_DIRECT_RECLAIM)) return NULL; pthread_mutex_lock(&cachep->lock); @@ -73,10 +73,17 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp) void *kmalloc(size_t size, gfp_t gfp) { - void *ret = malloc(size); + void *ret; + + if (!(gfp & __GFP_DIRECT_RECLAIM)) + return NULL; + + ret = malloc(size); uatomic_inc(&nr_allocated); if (kmalloc_verbose) printf("Allocating %p from malloc\n", ret); + if (gfp & __GFP_ZERO) + memset(ret, 0, size); return ret; } diff --git a/tools/testing/radix-tree/linux/compiler_types.h b/tools/testing/radix-tree/linux/compiler_types.h new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/testing/radix-tree/linux/gfp.h b/tools/testing/radix-tree/linux/gfp.h index e9fff59dfd8a..e3201ccf54c3 100644 --- a/tools/testing/radix-tree/linux/gfp.h +++ b/tools/testing/radix-tree/linux/gfp.h @@ -11,6 +11,7 @@ #define __GFP_IO 0x40u #define __GFP_FS 0x80u #define __GFP_NOWARN 0x200u +#define __GFP_ZERO 0x8000u #define __GFP_ATOMIC 0x80000u #define __GFP_ACCOUNT 0x100000u #define __GFP_DIRECT_RECLAIM 0x400000u diff --git a/tools/testing/radix-tree/linux/slab.h b/tools/testing/radix-tree/linux/slab.h index 979baeec7e70..a037def0dec6 100644 --- a/tools/testing/radix-tree/linux/slab.h +++ b/tools/testing/radix-tree/linux/slab.h @@ -3,6 +3,7 @@ #define SLAB_H #include +#include #define SLAB_HWCACHE_ALIGN 1 #define SLAB_PANIC 2 @@ -11,6 +12,11 @@ void *kmalloc(size_t size, gfp_t); void kfree(void *); +static inline void *kzalloc(size_t size, gfp_t gfp) +{ + return kmalloc(size, gfp | __GFP_ZERO); +} + void *kmem_cache_alloc(struct kmem_cache *cachep, int flags); void kmem_cache_free(struct kmem_cache *cachep, void *objp); -- cgit v1.2.3 From 192b2e742c06af399e8eecb4a1726520bfccece8 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Mon, 26 Feb 2018 13:17:07 +1100 Subject: selftests/powerpc: Skip tm-trap if transactional memory is not enabled Some processor revisions do not support transactional memory, and additionally kernel support can be disabled. In either case the tm-trap test should be skipped, otherwise it will fail with a SIGILL. Fixes: a08082f8e4e1 ("powerpc/selftests: Check endianness on trap in TM") Signed-off-by: Michael Ellerman --- tools/testing/selftests/powerpc/tm/tm-trap.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tools') diff --git a/tools/testing/selftests/powerpc/tm/tm-trap.c b/tools/testing/selftests/powerpc/tm/tm-trap.c index 5d92c23ee6cb..179d592f0073 100644 --- a/tools/testing/selftests/powerpc/tm/tm-trap.c +++ b/tools/testing/selftests/powerpc/tm/tm-trap.c @@ -255,6 +255,8 @@ int tm_trap_test(void) struct sigaction trap_sa; + SKIP_IF(!have_htm()); + trap_sa.sa_flags = SA_SIGINFO; trap_sa.sa_sigaction = trap_signal_handler; sigaction(SIGTRAP, &trap_sa, NULL); -- cgit v1.2.3 From 4b0ad07653ee94182e2d8f21404242c9e83ad0b4 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Mon, 26 Feb 2018 14:39:30 -0500 Subject: idr: Fix handling of IDs above INT_MAX Khalid reported that the kernel selftests are currently failing: selftests: test_bpf.sh ======================================== test_bpf: [FAIL] not ok 1..8 selftests: test_bpf.sh [FAIL] He bisected it to 6ce711f2750031d12cec91384ac5cfa0a485b60a ("idr: Make 1-based IDRs more efficient"). The root cause is doing a signed comparison in idr_alloc_u32() instead of an unsigned comparison. I went looking for any similar problems and found a couple (which would each result in the failure to warn in two situations that aren't supposed to happen). I knocked up a few test-cases to prove that I was right and added them to the test-suite. Reported-by: Khalid Aziz Tested-by: Khalid Aziz Signed-off-by: Matthew Wilcox --- lib/idr.c | 13 +++++----- tools/testing/radix-tree/idr-test.c | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) (limited to 'tools') diff --git a/lib/idr.c b/lib/idr.c index 99ec5bc89d25..823b813f08f8 100644 --- a/lib/idr.c +++ b/lib/idr.c @@ -36,8 +36,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid, { struct radix_tree_iter iter; void __rcu **slot; - int base = idr->idr_base; - int id = *nextid; + unsigned int base = idr->idr_base; + unsigned int id = *nextid; if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr))) return -EINVAL; @@ -204,10 +204,11 @@ int idr_for_each(const struct idr *idr, radix_tree_for_each_slot(slot, &idr->idr_rt, &iter, 0) { int ret; + unsigned long id = iter.index + base; - if (WARN_ON_ONCE(iter.index > INT_MAX)) + if (WARN_ON_ONCE(id > INT_MAX)) break; - ret = fn(iter.index + base, rcu_dereference_raw(*slot), data); + ret = fn(id, rcu_dereference_raw(*slot), data); if (ret) return ret; } @@ -230,8 +231,8 @@ void *idr_get_next(struct idr *idr, int *nextid) { struct radix_tree_iter iter; void __rcu **slot; - int base = idr->idr_base; - int id = *nextid; + unsigned long base = idr->idr_base; + unsigned long id = *nextid; id = (id < base) ? 0 : id - base; slot = radix_tree_iter_find(&idr->idr_rt, &iter, id); diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c index 44ef9eba5a7a..6c645eb77d42 100644 --- a/tools/testing/radix-tree/idr-test.c +++ b/tools/testing/radix-tree/idr-test.c @@ -178,6 +178,55 @@ void idr_get_next_test(int base) idr_destroy(&idr); } +int idr_u32_cb(int id, void *ptr, void *data) +{ + BUG_ON(id < 0); + BUG_ON(ptr != DUMMY_PTR); + return 0; +} + +void idr_u32_test1(struct idr *idr, u32 handle) +{ + static bool warned = false; + u32 id = handle; + int sid = 0; + void *ptr; + + BUG_ON(idr_alloc_u32(idr, DUMMY_PTR, &id, id, GFP_KERNEL)); + BUG_ON(id != handle); + BUG_ON(idr_alloc_u32(idr, DUMMY_PTR, &id, id, GFP_KERNEL) != -ENOSPC); + BUG_ON(id != handle); + if (!warned && id > INT_MAX) + printk("vvv Ignore these warnings\n"); + ptr = idr_get_next(idr, &sid); + if (id > INT_MAX) { + BUG_ON(ptr != NULL); + BUG_ON(sid != 0); + } else { + BUG_ON(ptr != DUMMY_PTR); + BUG_ON(sid != id); + } + idr_for_each(idr, idr_u32_cb, NULL); + if (!warned && id > INT_MAX) { + printk("^^^ Warnings over\n"); + warned = true; + } + BUG_ON(idr_remove(idr, id) != DUMMY_PTR); + BUG_ON(!idr_is_empty(idr)); +} + +void idr_u32_test(int base) +{ + DEFINE_IDR(idr); + idr_init_base(&idr, base); + idr_u32_test1(&idr, 10); + idr_u32_test1(&idr, 0x7fffffff); + idr_u32_test1(&idr, 0x80000000); + idr_u32_test1(&idr, 0x80000001); + idr_u32_test1(&idr, 0xffe00000); + idr_u32_test1(&idr, 0xffffffff); +} + void idr_checks(void) { unsigned long i; @@ -248,6 +297,9 @@ void idr_checks(void) idr_get_next_test(0); idr_get_next_test(1); idr_get_next_test(4); + idr_u32_test(4); + idr_u32_test(1); + idr_u32_test(0); } /* -- cgit v1.2.3 From 067b25a5639b10dfdd41ce6b4d4140fe84d0a8e7 Mon Sep 17 00:00:00 2001 From: Daniel Díaz Date: Wed, 7 Feb 2018 11:24:31 -0600 Subject: selftests/futex: Fix line continuation in Makefile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Makefile lacks a couple of line continuation backslashes in an `if' clause, which produces an error when make versions prior to 4.x are used for building the tests. $ make make[1]: Entering directory `/[...]/linux/tools/testing/selftests/futex' /bin/sh: -c: line 5: syntax error: unexpected end of file make[1]: *** [all] Error 1 make[1]: Leaving directory `/[...]/linux/tools/testing/selftests/futex' make: *** [all] Error 2 Signed-off-by: Daniel Díaz Signed-off-by: Shuah Khan --- tools/testing/selftests/futex/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile index cea4adcd42b8..a63e8453984d 100644 --- a/tools/testing/selftests/futex/Makefile +++ b/tools/testing/selftests/futex/Makefile @@ -12,9 +12,9 @@ all: BUILD_TARGET=$(OUTPUT)/$$DIR; \ mkdir $$BUILD_TARGET -p; \ make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\ - if [ -e $$DIR/$(TEST_PROGS) ]; then - rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/; - fi + if [ -e $$DIR/$(TEST_PROGS) ]; then \ + rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/; \ + fi \ done override define RUN_TESTS -- cgit v1.2.3 From 16c513b13477b8da7958e8112bf23cd59b87a7c1 Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Tue, 13 Feb 2018 10:45:57 -0700 Subject: selftests: memory-hotplug: silence test command echo Silence the following command being printed while running test. ./mem-on-off-test.sh -r 2 && echo "selftests: memory-hotplug [PASS]" || echo "selftests: memory-hotplug [FAIL]" Signed-off-by: Shuah Khan --- tools/testing/selftests/memory-hotplug/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/memory-hotplug/Makefile b/tools/testing/selftests/memory-hotplug/Makefile index 86636d207adf..183b46883875 100644 --- a/tools/testing/selftests/memory-hotplug/Makefile +++ b/tools/testing/selftests/memory-hotplug/Makefile @@ -4,7 +4,7 @@ all: include ../lib.mk TEST_PROGS := mem-on-off-test.sh -override RUN_TESTS := ./mem-on-off-test.sh -r 2 && echo "selftests: memory-hotplug [PASS]" || echo "selftests: memory-hotplug [FAIL]" +override RUN_TESTS := @./mem-on-off-test.sh -r 2 && echo "selftests: memory-hotplug [PASS]" || echo "selftests: memory-hotplug [FAIL]" override EMIT_TESTS := echo "$(RUN_TESTS)" run_full_test: -- cgit v1.2.3 From f6869826de700bce59e2cef14974f99836e34e4f Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Tue, 13 Feb 2018 10:48:29 -0700 Subject: selftests: vm: update .gitignore with new test Update .gitignore with new test. Signed-off-by: Shuah Khan --- tools/testing/selftests/vm/.gitignore | 1 + 1 file changed, 1 insertion(+) (limited to 'tools') diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore index 63c94d776e89..342c7bc9dc8c 100644 --- a/tools/testing/selftests/vm/.gitignore +++ b/tools/testing/selftests/vm/.gitignore @@ -11,3 +11,4 @@ mlock-intersect-test mlock-random-test virtual_address_range gup_benchmark +va_128TBswitch -- cgit v1.2.3 From b7abbd5a3533a31a1e7d4696ea275df543440c51 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Wed, 28 Feb 2018 15:15:56 +1100 Subject: selftests/powerpc: Fix missing clean of pmu/lib.o The tm-resched-dscr test links against pmu/lib.o, but we don't have a rule to clean pmu/lib.o. This can lead to a build break if you build for big endian and then little, or vice versa. Fix it by making tm-resched-dscr depend on pmu/lib.c, causing the code to be built directly in, meaning no .o is generated. Signed-off-by: Michael Ellerman --- tools/testing/selftests/powerpc/tm/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile index a23453943ad2..5c72ff978f27 100644 --- a/tools/testing/selftests/powerpc/tm/Makefile +++ b/tools/testing/selftests/powerpc/tm/Makefile @@ -16,7 +16,7 @@ $(OUTPUT)/tm-syscall: tm-syscall-asm.S $(OUTPUT)/tm-syscall: CFLAGS += -I../../../../../usr/include $(OUTPUT)/tm-tmspr: CFLAGS += -pthread $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64 -$(OUTPUT)/tm-resched-dscr: ../pmu/lib.o +$(OUTPUT)/tm-resched-dscr: ../pmu/lib.c $(OUTPUT)/tm-unavailable: CFLAGS += -O0 -pthread -m64 -Wno-error=uninitialized -mvsx $(OUTPUT)/tm-trap: CFLAGS += -O0 -pthread -m64 -- cgit v1.2.3 From cd4a6f3ab4d80cb919d15897eb3cbc85c2009d4b Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Mon, 26 Feb 2018 15:22:22 +1100 Subject: selftests/powerpc: Skip the subpage_prot tests if the syscall is unavailable The subpage_prot syscall is only functional when the system is using the Hash MMU. Since commit 5b2b80714796 ("powerpc/mm: Invalidate subpage_prot() system call on radix platforms") it returns ENOENT when the Radix MMU is active. Currently this just makes the test fail. Additionally the syscall is not available if the kernel is built with 4K pages, or if CONFIG_PPC_SUBPAGE_PROT=n, in which case it returns ENOSYS because the syscall is missing entirely. So check explicitly for ENOENT and ENOSYS and skip if we see either of those. Signed-off-by: Michael Ellerman --- tools/testing/selftests/powerpc/mm/subpage_prot.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'tools') diff --git a/tools/testing/selftests/powerpc/mm/subpage_prot.c b/tools/testing/selftests/powerpc/mm/subpage_prot.c index 35ade7406dcd..3ae77ba93208 100644 --- a/tools/testing/selftests/powerpc/mm/subpage_prot.c +++ b/tools/testing/selftests/powerpc/mm/subpage_prot.c @@ -135,6 +135,16 @@ static int run_test(void *addr, unsigned long size) return 0; } +static int syscall_available(void) +{ + int rc; + + errno = 0; + rc = syscall(__NR_subpage_prot, 0, 0, 0); + + return rc == 0 || (errno != ENOENT && errno != ENOSYS); +} + int test_anon(void) { unsigned long align; @@ -145,6 +155,8 @@ int test_anon(void) void *mallocblock; unsigned long mallocsize; + SKIP_IF(!syscall_available()); + if (getpagesize() != 0x10000) { fprintf(stderr, "Kernel page size must be 64K!\n"); return 1; @@ -180,6 +192,8 @@ int test_file(void) off_t filesize; int fd; + SKIP_IF(!syscall_available()); + fd = open(file_name, O_RDWR); if (fd == -1) { perror("failed to open file"); -- cgit v1.2.3 From ba004a2955f759946d8c98ca1a9c8d09818b1223 Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Thu, 1 Mar 2018 13:04:28 -0700 Subject: selftests: memory-hotplug: fix emit_tests regression Commit 16c513b13477 ("selftests: memory-hotplug: silence test command echo") introduced regression in emit_tests and results in the following failure when selftests are installed and run. Fix it. Running tests in memory-hotplug ======================================== ./run_kselftest.sh: line 121: @./mem-on-off-test.sh: No such file or directory selftests: memory-hotplug [FAIL] Fixes: 16c513b13477 (selftests: memory-hotplug: silence test command echo") Reported-by: Naresh Kamboju Tested-by: Anders Roxell Signed-off-by: Shuah Khan --- tools/testing/selftests/memory-hotplug/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/memory-hotplug/Makefile b/tools/testing/selftests/memory-hotplug/Makefile index 183b46883875..686da510f989 100644 --- a/tools/testing/selftests/memory-hotplug/Makefile +++ b/tools/testing/selftests/memory-hotplug/Makefile @@ -5,7 +5,8 @@ include ../lib.mk TEST_PROGS := mem-on-off-test.sh override RUN_TESTS := @./mem-on-off-test.sh -r 2 && echo "selftests: memory-hotplug [PASS]" || echo "selftests: memory-hotplug [FAIL]" -override EMIT_TESTS := echo "$(RUN_TESTS)" + +override EMIT_TESTS := echo "$(subst @,,$(RUN_TESTS))" run_full_test: @/bin/bash ./mem-on-off-test.sh && echo "memory-hotplug selftests: [PASS]" || echo "memory-hotplug selftests: [FAIL]" -- cgit v1.2.3 From 79f3a8e662c1ae6e85737eca9ae7d6b52cf87815 Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Fri, 2 Mar 2018 14:44:39 +0100 Subject: tc-testing: skbmod: fix match value of ethertype iproute2 print_skbmod() prints the configured ethertype using format 0x%X: therefore, test 9aa8 systematically fails, because it configures action #4 using ethertype 0x0031, and expects 0x0031 when it reads it back. Changing the expected value to 0x31 lets the test result 'not ok' become 'ok'. tested with: # ./tdc.py -e 9aa8 Test 9aa8: Get a single skbmod action from a list All test results: 1..1 ok 1 9aa8 Get a single skbmod action from a list Fixes: cf797ac49b94 ("tc-testing: Add test cases for police and skbmod") Signed-off-by: Davide Caratti Signed-off-by: David S. Miller --- tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json index e34075059c26..90bba48c3f07 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbmod.json @@ -315,7 +315,7 @@ "cmdUnderTest": "$TC actions ls action skbmod", "expExitCode": "0", "verifyCmd": "$TC actions get action skbmod index 4", - "matchPattern": "action order [0-9]*: skbmod pipe set etype 0x0031", + "matchPattern": "action order [0-9]*: skbmod pipe set etype 0x31", "matchCount": "1", "teardown": [ "$TC actions flush action skbmod" -- cgit v1.2.3 From f6d3f35e006496c282ccbb67494d90b04f6cba10 Mon Sep 17 00:00:00 2001 From: Sangwon Hong Date: Mon, 12 Feb 2018 04:37:44 +0900 Subject: perf kallsyms: Fix the usage on the man page First, all man pages highlight only perf and subcommands except 'perf kallsyms', which includes the full usage. Fix it for commands to monopolize underlines. Second, options can be ommited when executing 'perf kallsyms', so add square brackets between