CONFIG_CGROUP_BPF=y is required for test_dev_cgroup test case.
Signed-off-by: Naresh Kamboju <naresh.kamboju(a)linaro.org>
---
tools/testing/selftests/bpf/config | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 52d53ed..9d48973 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -3,3 +3,4 @@ CONFIG_BPF_SYSCALL=y
CONFIG_NET_CLS_BPF=m
CONFIG_BPF_EVENTS=y
CONFIG_TEST_BPF=m
+CONFIG_CGROUP_BPF=y
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
CONFIG_CGROUP_BPF=y is required for test_dev_cgroup test case.
Signed-off-by: Naresh Kamboju <naresh.kamboju(a)linaro.org>
---
tools/testing/selftests/net/config | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index e57b4ac..02301c6 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -1,3 +1,4 @@
CONFIG_USER_NS=y
CONFIG_BPF_SYSCALL=y
CONFIG_TEST_BPF=m
+CONFIG_CGROUP_BPF=y
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello,
I came along some software modules where I suggested source code adjustments.
Example:
nfs/write: Use common error handling code in nfs_lock_and_join_requests()
https://lkml.org/lkml/2017/11/7/599https://patchwork.kernel.org/patch/10047013/https://lkml.kernel.org/r/<7f072f78-eef4-6d87-d233-cee71dac5a32(a)users.sourceforge.net>
I would like to check corresponding build results then without extra
optimisation applied by the compiler.
But I got surprised by error messages for a command like the following.
elfring@Sonne:~/Projekte/Linux/next-patched> my_cc=/usr/bin/gcc-7 && LANG=C make -j4 CC="${my_cc}" HOSTCC="${my_cc}" EXTRA_CFLAGS='-O0' allmodconfig fs/nfs/write.o
…
In file included from ./include/linux/compiler.h:58:0,
from ./include/uapi/linux/stddef.h:1,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from fs/nfs/write.c:9:
./arch/x86/include/asm/jump_label.h: In function ‘trace_nfs_writeback_page_enter’:
./include/linux/compiler-gcc.h:275:38: warning: asm operand 0 probably doesn’t match constraints
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
…
How do you think about to improve this software situation anyhow?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Perf tool bpf selftests revealed a broken uapi for s390 and arm64.
With the BPF_PROG_TYPE_PERF_EVENT program type the bpf_perf_event
structure exports the pt_regs structure for all architectures.
This fails for s390 and arm64 because pt_regs are not part of the
user api and kept in-kernel only. To mitigate the broken uapi,
introduce a wrapper that exports pt_regs in an asm-generic way.
For arm64, export the exising user_pt_regs structure. For s390,
introduce a user_pt_regs structure that exports the beginning of
pt_regs.
Note that user_pt_regs must export from the beginning of pt_regs
as BPF_PROG_TYPE_PERF_EVENT program type is not the only type for
running BPF programs.
Some more background:
For the bpf_perf_event, there is a uapi definition that is
passed to the BPF program. For other "probe" points like
trace points, kprobes, and uprobes, there is no uapi and the
BPF program is always passed pt_regs (which is OK as the BPF
program runs in the kernel context). The perf tool can attach
BPF programs to all of these "probe" points and, optionally,
can create a BPF prologue to access particular arguments
(passed as registers). For this, it uses DWARF/CFI
information to obtain the register and calls a perf-arch
backend function, regs_query_register_offset(). This function
returns the index into (user_)pt_regs for a particular
register. Then, perf creates a BPF prologue that accesses
this register based on the passed stucture from the "probe"
point.
Part of this series, are also updates to the testing and bpf selftest
to deal with asm-specifics. To complete the bpf support in perf, the
the regs_query_register_offset function is added for s390 to support
BPF prologue creation.
Hendrik Brueckner (5):
bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
type
arm64/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
type
selftests/bpf: sync kernel headers and introduce arch support in
Makefile
perf s390: add regs_query_register_offset()
arch/arm64/include/asm/perf_event.h | 2 +
arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +
arch/s390/include/asm/perf_event.h | 1 +
arch/s390/include/asm/ptrace.h | 11 +-
arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +
arch/s390/include/uapi/asm/ptrace.h | 11 +
include/linux/perf_event.h | 6 +-
include/uapi/asm-generic/bpf_perf_event.h | 9 +
include/uapi/linux/bpf_perf_event.h | 5 +-
kernel/events/core.c | 2 +-
tools/arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/ptrace.h | 471 +++++++++++++++++++++
tools/include/uapi/asm-generic/bpf_perf_event.h | 9 +
tools/include/uapi/linux/bpf_perf_event.h | 6 +-
tools/perf/arch/s390/Makefile | 1 +
tools/perf/arch/s390/util/dwarf-regs.c | 32 +-
tools/perf/check-headers.sh | 1 +
tools/testing/selftests/bpf/Makefile | 14 +-
19 files changed, 602 insertions(+), 15 deletions(-)
create mode 100644 arch/arm64/include/uapi/asm/bpf_perf_event.h
create mode 100644 arch/s390/include/uapi/asm/bpf_perf_event.h
create mode 100644 include/uapi/asm-generic/bpf_perf_event.h
create mode 100644 tools/arch/arm64/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/ptrace.h
create mode 100644 tools/include/uapi/asm-generic/bpf_perf_event.h
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello,
I have constructed another demonstration program.
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
int main(void)
{
FILE *f = fopen("/dev/full", "a");
if (!f)
goto report_failure;
{
int const c = 'X';
if (fputc(c, f) != c)
goto report_failure;
}
return EXIT_SUCCESS;
report_failure:
perror(__func__);
return errno;
}
I got the following result.
elfring@Sonne:~/Projekte/selftests> gcc-7 putc_into_full_file1.c && ./a.out; echo $?
0
Does such a simple test example need further software development considerations?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
We have found "no MPX support" bug while running kselftests on
SuperServer 5019S-ML - x86_64 device.
Please provide your comments to fix this bug.
LKFT: 4.4-rc 4.9-rc 4.13-rc 4.14-rc: x86: kselftest mpx-mini-test_64 -
no MPX support - failed - 3869 Aborted (core dumped) (edit)
https://bugs.linaro.org/show_bug.cgi?id=3497
NOTE:
Please add your self to Bugzilla to add comments.
Best regards
Naresh Kamboju
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
These patches are fixing a bug and improve testcase to
ensure adding 256 kprobe events for test.
The 1st patch is clearly a bug, so it should be fixed
in stable kernel too.
Thank you,
---
Masami Hiramatsu (2):
[BUGFIX] selftest: ftrace: Fix to pick text symbols for kprobes
selftest: ftrace: Fix to add 256 kprobe events correctly
.../ftrace/test.d/kprobe/multiple_kprobes.tc | 21 +++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat(a)kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 21, 2017 at 09:18:53AM -0500, Mathieu Desnoyers wrote:
> +int percpu_list_push(struct percpu_list *list, struct percpu_list_node *node)
> +{
> + intptr_t *targetptr, newval, expect;
> + int cpu, ret;
> +
> + /* Try fast path. */
> + cpu = rseq_cpu_start();
> + /* Load list->c[cpu].head with single-copy atomicity. */
> + expect = (intptr_t)READ_ONCE(list->c[cpu].head);
> + newval = (intptr_t)node;
> + targetptr = (intptr_t *)&list->c[cpu].head;
> + node->next = (struct percpu_list_node *)expect;
> + ret = rseq_cmpeqv_storev(targetptr, expect, newval, cpu);
> + if (likely(!ret))
> + return cpu;
> + return cpu;
> +}
> +static inline __attribute__((always_inline))
> +int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv,
> + int cpu)
> +{
> + __asm__ __volatile__ goto (
> + RSEQ_ASM_DEFINE_TABLE(3, __rseq_table, 0x0, 0x0, 1f, 2f-1f, 4f)
> + RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
> + RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
So the actual C part of the RSEQ is subject to an ABA, right? We can get
migrated to another CPU and back again without then failing here.
It used to be that this was caught by the sequence count, but that is
now gone.
The thing that makes it work is the compare against @v:
> + "cmpq %[v], %[expect]\n\t"
> + "jnz 5f\n\t"
That then ensures things are still as we observed them before (although
this itself is also subject to ABA).
This means all RSEQ primitives that have a C part must have a cmp-and-
form, but I suppose that was already pretty much the case anyway. I just
don't remember seeing that spelled out anywhere. Then again, I've not
yet read that manpage.
> + /* final store */
> + "movq %[newv], %[v]\n\t"
> + "2:\n\t"
> + RSEQ_ASM_DEFINE_ABORT(4, __rseq_failure, RSEQ_SIG, "", abort)
> + RSEQ_ASM_DEFINE_CMPFAIL(5, __rseq_failure, "", cmpfail)
> + : /* gcc asm goto does not allow outputs */
> + : [cpu_id]"r"(cpu),
> + [current_cpu_id]"m"(__rseq_abi.cpu_id),
> + [rseq_cs]"m"(__rseq_abi.rseq_cs),
> + [v]"m"(*v),
> + [expect]"r"(expect),
> + [newv]"r"(newv)
> + : "memory", "cc", "rax"
> + : abort, cmpfail
> + );
> + return 0;
> +abort:
> + return -1;
> +cmpfail:
> + return 1;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 21, 2017 at 09:18:53AM -0500, Mathieu Desnoyers wrote:
> diff --git a/tools/testing/selftests/rseq/rseq-x86.h b/tools/testing/selftests/rseq/rseq-x86.h
> new file mode 100644
> index 000000000000..63e81d6c61fa
> --- /dev/null
> +++ b/tools/testing/selftests/rseq/rseq-x86.h
> @@ -0,0 +1,898 @@
> +/*
> + * rseq-x86.h
> + *
> + * (C) Copyright 2016 - Mathieu Desnoyers <mathieu.desnoyers(a)efficios.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <stdint.h>
> +
> +#define RSEQ_SIG 0x53053053
> +
> +#ifdef __x86_64__
> +
> +#define rseq_smp_mb() __asm__ __volatile__ ("mfence" : : : "memory")
See commit:
450cbdd0125c ("locking/x86: Use LOCK ADD for smp_mb() instead of MFENCE")
> +#define rseq_smp_rmb() barrier()
> +#define rseq_smp_wmb() barrier()
> +
> +#define rseq_smp_load_acquire(p) \
> +__extension__ ({ \
> + __typeof(*p) ____p1 = RSEQ_READ_ONCE(*p); \
> + barrier(); \
> + ____p1; \
> +})
> +
> +#define rseq_smp_acquire__after_ctrl_dep() rseq_smp_rmb()
> +
> +#define rseq_smp_store_release(p, v) \
> +do { \
> + barrier(); \
> + RSEQ_WRITE_ONCE(*p, v); \
> +} while (0)
> +
> +#define RSEQ_ASM_DEFINE_TABLE(label, section, version, flags, \
> + start_ip, post_commit_offset, abort_ip) \
> + ".pushsection " __rseq_str(section) ", \"aw\"\n\t" \
> + ".balign 32\n\t" \
> + __rseq_str(label) ":\n\t" \
> + ".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
> + ".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
> + ".popsection\n\t"
OK, so this creates table entry, but why is @section an argument, AFAICT
its _always_ the same thing, no?
> +#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
> + RSEQ_INJECT_ASM(1) \
> + "leaq " __rseq_str(cs_label) "(%%rip), %%rax\n\t" \
> + "movq %%rax, %[" __rseq_str(rseq_cs) "]\n\t" \
> + __rseq_str(label) ":\n\t"
And this sets the TLS variable to point to the table entry from the
previous macro, no? But again @rseq_cs seems to always be the very same,
why is that an argument?
> +#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
> + RSEQ_INJECT_ASM(2) \
> + "cmpl %[" __rseq_str(cpu_id) "], %[" __rseq_str(current_cpu_id) "]\n\t" \
> + "jnz " __rseq_str(label) "\n\t"
more things that are always the same it seems..
> +#define RSEQ_ASM_DEFINE_ABORT(label, section, sig, teardown, abort_label) \
> + ".pushsection " __rseq_str(section) ", \"ax\"\n\t" \
> + /* Disassembler-friendly signature: nopl <sig>(%rip). */\
> + ".byte 0x0f, 0x1f, 0x05\n\t" \
> + ".long " __rseq_str(sig) "\n\t" \
> + __rseq_str(label) ":\n\t" \
> + teardown \
> + "jmp %l[" __rseq_str(abort_label) "]\n\t" \
> + ".popsection\n\t"
@section and @sig seem to always be the same...
> +#define RSEQ_ASM_DEFINE_CMPFAIL(label, section, teardown, cmpfail_label) \
> + ".pushsection " __rseq_str(section) ", \"ax\"\n\t" \
> + __rseq_str(label) ":\n\t" \
> + teardown \
> + "jmp %l[" __rseq_str(cmpfail_label) "]\n\t" \
> + ".popsection\n\t"
Somewhat failing to see the point of this macro, it seems to just
obfuscate the normal failure path.
> +static inline __attribute__((always_inline))
> +int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv,
> + int cpu)
I find this a very confusing name for what is essentially
compare-and-exchange or compare-and-swap, no?
> +{
> + __asm__ __volatile__ goto (
> + RSEQ_ASM_DEFINE_TABLE(3, __rseq_table, 0x0, 0x0, 1f, 2f-1f, 4f)
So we set up the section, but unreadably so... reducing the number of
arguments would help a lot.
Rename the current one to __RSEQ_ASM_DEFINE_TABLE() and then use:
#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
__RSEQ_ASM_DEFINE_TABLE(label, __rseq_table, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
or something, such that we can write:
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
> + RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
And here we open start the rseq by storing the table entry pointer into
the TLS thingy.
> + RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
> + "cmpq %[v], %[expect]\n\t"
> + "jnz 5f\n\t"
"jnz %l[cmpfail]\n\t"
was too complicated?
> + /* final store */
> + "movq %[newv], %[v]\n\t"
> + "2:\n\t"
> + RSEQ_ASM_DEFINE_ABORT(4, __rseq_failure, RSEQ_SIG, "", abort)
> + RSEQ_ASM_DEFINE_CMPFAIL(5, __rseq_failure, "", cmpfail)
> + : /* gcc asm goto does not allow outputs */
> + : [cpu_id]"r"(cpu),
> + [current_cpu_id]"m"(__rseq_abi.cpu_id),
> + [rseq_cs]"m"(__rseq_abi.rseq_cs),
> + [v]"m"(*v),
> + [expect]"r"(expect),
> + [newv]"r"(newv)
: [cpu_id] "r" (cpu),
[current_cpu_id] "m" (__rseq_abi.cpu_id),
[rseq_cs] "m" (__rseq_abi.rseq_cs),
[v] "m" (*v),
[expect] "r" (expect),
[newv] "r" (newv)
or something does read much better
> + : "memory", "cc", "rax"
> + : abort, cmpfail
> + );
> + return 0;
> +abort:
> + return -1;
> +cmpfail:
> + return 1;
> +}
> +
> +static inline __attribute__((always_inline))
> +int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
> + off_t voffp, intptr_t *load, int cpu)
so this thing does what now? It compares @v to @expectnot, when _not_
matching it will store @voffp into @v and load something..?
> +{
> + __asm__ __volatile__ goto (
> + RSEQ_ASM_DEFINE_TABLE(3, __rseq_table, 0x0, 0x0, 1f, 2f-1f, 4f)
> + RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
> + RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
> + "cmpq %[v], %[expectnot]\n\t"
> + "jz 5f\n\t"
So I would prefer "je" in this context, or rather:
je %l[cmpfail]
> + "movq %[v], %%rax\n\t"
loads @v in A
But it could already have changed since the previous load from cmp, no?
Would it not make sense to put this load before the cmp and use A
instead?
> + "movq %%rax, %[load]\n\t"
stores A in @load
> + "addq %[voffp], %%rax\n\t"
adds @off to A
> + "movq (%%rax), %%rax\n\t"
loads (A) in A
> + /* final store */
> + "movq %%rax, %[v]\n\t"
stores A in @v
So the whole thing loads @v into @load, adds and offset, dereferences
and adds that back in @v, provided @v doesn't match @expected.. whee.
> + "2:\n\t"
> + RSEQ_ASM_DEFINE_ABORT(4, __rseq_failure, RSEQ_SIG, "", abort)
> + RSEQ_ASM_DEFINE_CMPFAIL(5, __rseq_failure, "", cmpfail)
> + : /* gcc asm goto does not allow outputs */
> + : [cpu_id]"r"(cpu),
> + [current_cpu_id]"m"(__rseq_abi.cpu_id),
> + [rseq_cs]"m"(__rseq_abi.rseq_cs),
> + /* final store input */
> + [v]"m"(*v),
> + [expectnot]"r"(expectnot),
> + [voffp]"er"(voffp),
> + [load]"m"(*load)
> + : "memory", "cc", "rax"
> + : abort, cmpfail
> + );
> + return 0;
> +abort:
> + return -1;
> +cmpfail:
> + return 1;
> +}
> +#elif __i386__
> +
> +/*
> + * Support older 32-bit architectures that do not implement fence
> + * instructions.
> + */
> +#define rseq_smp_mb() \
> + __asm__ __volatile__ ("lock; addl $0,0(%%esp)" : : : "memory")
> +#define rseq_smp_rmb() \
> + __asm__ __volatile__ ("lock; addl $0,0(%%esp)" : : : "memory")
> +#define rseq_smp_wmb() \
> + __asm__ __volatile__ ("lock; addl $0,0(%%esp)" : : : "memory")
Oh shiny, you're supporting that OOSTORE and PPRO_FENCE nonsense?
Going by commit:
09df7c4c8097 ("x86: Remove CONFIG_X86_OOSTORE")
That smp_wmb() one was an 'optimization' (forced store buffer flush) but
not a correctness thing. And we dropped that stuff from the kernel a
_long_ time ago.
Ideally we'd kill that PPRO_FENCE crap too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 21, 2017 at 09:18:53AM -0500, Mathieu Desnoyers wrote:
> Implements two basic tests of RSEQ functionality, and one more
> exhaustive parameterizable test.
>
> The first, "basic_test" only asserts that RSEQ works moderately
> correctly. E.g. that the CPUID pointer works.
>
> "basic_percpu_ops_test" is a slightly more "realistic" variant,
> implementing a few simple per-cpu operations and testing their
> correctness.
>
> "param_test" is a parametrizable restartable sequences test. See
> the "--help" output for usage.
>
> A run_param_test.sh script runs many variants of the parametrizable
> tests.
>
> As part of those tests, a helper library "rseq" implements a user-space
> API around restartable sequences. It uses the cpu_opv system call as
> fallback when single-stepped by a debugger. It exposes the instruction
> pointer addresses where the rseq assembly blocks begin and end, as well
> as the associated abort instruction pointer, in the __rseq_table
> section. This section allows debuggers may know where to place
> breakpoints when single-stepping through assembly blocks which may be
> aborted at any point by the kernel.
Could I ask you to split this in smaller bits?
I'd start with just the rseq library, using only the rseq interface.
Then add the whole cpu_opv fallback stuff.
Then add the selftests using librseq.
As is this is a tad much to read in a single go.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/21/2017 03:19 PM, Mathieu Desnoyers wrote:
> Implements two basic tests of RSEQ functionality, and one more
> exhaustive parameterizable test.
>
> The first, "basic_test" only asserts that RSEQ works moderately
> correctly. E.g. that the CPUID pointer works.
>
> "basic_percpu_ops_test" is a slightly more "realistic" variant,
> implementing a few simple per-cpu operations and testing their
> correctness.
>
> "param_test" is a parametrizable restartable sequences test. See
> the "--help" output for usage.
>
> A run_param_test.sh script runs many variants of the parametrizable
> tests.
>
> As part of those tests, a helper library "rseq" implements a user-space
> API around restartable sequences. It uses the cpu_opv system call as
> fallback when single-stepped by a debugger. It exposes the instruction
> pointer addresses where the rseq assembly blocks begin and end, as well
> as the associated abort instruction pointer, in the __rseq_table
> section. This section allows debuggers may know where to place
> breakpoints when single-stepping through assembly blocks which may be
> aborted at any point by the kernel.
>
> The rseq library expose APIs that present the fast-path operations.
> The new from userspace is, e.g. for a counter increment:
>
> cpu = rseq_cpu_start();
> ret = rseq_addv(&data->c[cpu].count, 1, cpu);
> if (likely(!ret))
> return 0; /* Success. */
> do {
> cpu = rseq_current_cpu();
> ret = cpu_op_addv(&data->c[cpu].count, 1, cpu);
> if (likely(!ret))
> return 0; /* Success. */
> } while (ret > 0 || errno == EAGAIN);
> perror("cpu_op_addv");
> return -1; /* Unexpected error. */
>
> PowerPC tests have been implemented by Boqun Feng.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers(a)efficios.com>
> CC: Russell King <linux(a)arm.linux.org.uk>
> CC: Catalin Marinas <catalin.marinas(a)arm.com>
> CC: Will Deacon <will.deacon(a)arm.com>
> CC: Thomas Gleixner <tglx(a)linutronix.de>
> CC: Paul Turner <pjt(a)google.com>
> CC: Andrew Hunter <ahh(a)google.com>
> CC: Peter Zijlstra <peterz(a)infradead.org>
> CC: Andy Lutomirski <luto(a)amacapital.net>
> CC: Andi Kleen <andi(a)firstfloor.org>
> CC: Dave Watson <davejwatson(a)fb.com>
> CC: Chris Lameter <cl(a)linux.com>
> CC: Ingo Molnar <mingo(a)redhat.com>
> CC: "H. Peter Anvin" <hpa(a)zytor.com>
> CC: Ben Maurer <bmaurer(a)fb.com>
> CC: Steven Rostedt <rostedt(a)goodmis.org>
> CC: "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com>
> CC: Josh Triplett <josh(a)joshtriplett.org>
> CC: Linus Torvalds <torvalds(a)linux-foundation.org>
> CC: Andrew Morton <akpm(a)linux-foundation.org>
> CC: Boqun Feng <boqun.feng(a)gmail.com>
> CC: Shuah Khan <shuah(a)kernel.org>
> CC: linux-kselftest(a)vger.kernel.org
> CC: linux-api(a)vger.kernel.org
> ---
> Changes since v1:
> - Provide abort-ip signature: The abort-ip signature is located just
> before the abort-ip target. It is currently hardcoded, but a
> user-space application could use the __rseq_table to iterate on all
> abort-ip targets and use a random value as signature if needed in the
> future.
> - Add rseq_prepare_unload(): Libraries and JIT code using rseq critical
> sections need to issue rseq_prepare_unload() on each thread at least
> once before reclaim of struct rseq_cs.
> - Use initial-exec TLS model, non-weak symbol: The initial-exec model is
> signal-safe, whereas the global-dynamic model is not. Remove the
> "weak" symbol attribute from the __rseq_abi in rseq.c. The rseq.so
> library will have ownership of that symbol, and there is not reason for
> an application or user library to try to define that symbol.
> The expected use is to link against libreq.so, which owns and provide
> that symbol.
> - Set cpu_id to -2 on register error
> - Add rseq_len syscall parameter, rseq_cs version
> - Ensure disassember-friendly signature: x86 32/64 disassembler have a
> hard time decoding the instruction stream after a bad instruction. Use
> a nopl instruction to encode the signature. Suggested by Andy Lutomirski.
> - Exercise parametrized tests variants in a shell scripts.
> - Restartable sequences selftests: Remove use of event counter.
> - Use cpu_id_start field: With the cpu_id_start field, the C
> preparation phase of the fast-path does not need to compare cpu_id < 0
> anymore.
> - Signal-safe registration and refcounting: Allow libraries using
> librseq.so to register it from signal handlers.
> - Use OVERRIDE_TARGETS in makefile.
> - Use "m" constraints for rseq_cs field.
>
> Changes since v2:
> - Update based on Thomas Gleixner's comments.
>
> Changes since v3:
> - Generate param_test_skip_fastpath and param_test_benchmark with
> -DSKIP_FASTPATH and -DBENCHMARK (respectively). Add param_test_fastpath
> to run_param_test.sh.
> ---
> MAINTAINERS | 1 +
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/rseq/.gitignore | 4 +
> tools/testing/selftests/rseq/Makefile | 33 +
> .../testing/selftests/rseq/basic_percpu_ops_test.c | 333 +++++
> tools/testing/selftests/rseq/basic_test.c | 55 +
> tools/testing/selftests/rseq/param_test.c | 1285 ++++++++++++++++++++
> tools/testing/selftests/rseq/rseq-arm.h | 535 ++++++++
> tools/testing/selftests/rseq/rseq-ppc.h | 567 +++++++++
> tools/testing/selftests/rseq/rseq-x86.h | 898 ++++++++++++++
> tools/testing/selftests/rseq/rseq.c | 116 ++
> tools/testing/selftests/rseq/rseq.h | 154 +++
> tools/testing/selftests/rseq/run_param_test.sh | 126 ++
> 13 files changed, 4108 insertions(+)
> create mode 100644 tools/testing/selftests/rseq/.gitignore
> create mode 100644 tools/testing/selftests/rseq/Makefile
> create mode 100644 tools/testing/selftests/rseq/basic_percpu_ops_test.c
> create mode 100644 tools/testing/selftests/rseq/basic_test.c
> create mode 100644 tools/testing/selftests/rseq/param_test.c
> create mode 100644 tools/testing/selftests/rseq/rseq-arm.h
> create mode 100644 tools/testing/selftests/rseq/rseq-ppc.h
> create mode 100644 tools/testing/selftests/rseq/rseq-x86.h
> create mode 100644 tools/testing/selftests/rseq/rseq.c
> create mode 100644 tools/testing/selftests/rseq/rseq.h
> create mode 100755 tools/testing/selftests/rseq/run_param_test.sh
>
Looks good.
Acked-by: Shuah Khan <shuahkh(a)osg.samsung.com>
thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
----- On Nov 21, 2017, at 10:34 AM, shuah shuah(a)kernel.org wrote:
[...]
>> ---
>> MAINTAINERS | 1 +
>> tools/testing/selftests/Makefile | 1 +
>> tools/testing/selftests/rseq/.gitignore | 4 +
>
> Thanks for the .gitignore files. It is commonly missed change, I end
> up adding one to clean things up after tests get in.
I'm used to receive patches where contributors forget to add new files
to gitignore within my own projects, which may contribute to my awareness
of this pain point. :)
[...]
>> +
>> +void *test_percpu_inc_thread(void *arg)
>> +{
>> + struct inc_thread_test_data *thread_data = arg;
>> + struct inc_test_data *data = thread_data->data;
>> + long long i, reps;
>> +
>> + if (!opt_disable_rseq && thread_data->reg
>> + && rseq_register_current_thread())
>> + abort();
>> + reps = thread_data->reps;
>> + for (i = 0; i < reps; i++) {
>> + int cpu, ret;
>> +
>> +#ifndef SKIP_FASTPATH
>> + /* Try fast path. */
>> + cpu = rseq_cpu_start();
>> + ret = rseq_addv(&data->c[cpu].count, 1, cpu);
>> + if (likely(!ret))
>> + goto next;
>> +#endif
>
> So the test needs to compiled with this enabled? I think it would be better
> to make this an argument to be abel to select at test start time as opposed
> to making this compile time option. Remember that these tests get run in
> automated test rings. Making this a compile time otpion pertty much ensures
> that this path will not be tested.
>
> So I would reccommend adding a paratemer.
>
>> + slowpath:
>> + __attribute__((unused));
>> + for (;;) {
>> + /* Fallback on cpu_opv system call. */
>> + cpu = rseq_current_cpu();
>> + ret = cpu_op_addv(&data->c[cpu].count, 1, cpu);
>> + if (likely(!ret))
>> + break;
>> + assert(ret >= 0 || errno == EAGAIN);
>> + }
>> + next:
>> + __attribute__((unused));
>> +#ifndef BENCHMARK
>> + if (i != 0 && !(i % (reps / 10)))
>> + printf_verbose("tid %d: count %lld\n", (int) gettid(), i);
>> +#endif
>
> Same comment as before. Avoid compile time options.
The goal of those compiler define are to generate the altered code without
adding branches into the fast-paths.
Here is an alternative solution that should take care of your concern: I'll
build multiple targets for param_test.c:
param_test
param_test_skip_fastpath (built with -DSKIP_FASTPATH)
param_test_benchmark (build with -DBENCHMARK)
I'll update run_param_test.sh to run both param_test and param_test_skip_fastpath.
Note that "param_test_benchmark" is only useful for benchmarking,
so I don't plan to run it from run_param_test.sh which is meant
to track regressions.
Is that approach OK with you ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Markus Elfring <elfring(a)users.sourceforge.net>
Date: Tue, 21 Nov 2017 20:56:51 +0100
Use the data type "sig_atomic_t" for the variable "done"
so that it can be safely modified by a signal handler.
Fixes: 0bc4b0cf15708fca04095232c4e448634e94d029 ("selftests: add basic posix timers selftests")
Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net>
---
tools/testing/selftests/timers/posix_timers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index 15cf56d32155..d64732c8b69a 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -20,7 +20,7 @@
#define DELAY 2
#define USECS_PER_SEC 1000000
-static volatile int done;
+static sig_atomic_t done;
/* Busy loop in userspace to elapse ITIMER_VIRTUAL */
static void user_loop(void)
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello,
Static source code analysis points out that the checking of return values from
some function calls is incomplete also in this software area.
How would you like to fix remaining open issues there?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
At Linaro we’ve been putting effort into regularly running kernel tests over
arm, arm64 and x86_64 targets. On those targets we’re running mainline, -next,
4.4, and 4.9 kernels and yes we are adding to this list as the hardware
capacity grows.
For test buckets we’re using just LTP, kselftest and libhugetlbfs and
like kernels we will add to this list.
With the 4.14 cycle being a little ‘different’ in so much as the goal to
have it be an LTS kernel I think it’s important to take a look at some
4.14 test results.
Grab a beverage, this is a bit of a long post. But quick summery 4.14 as
released looks just as good as 4.13, for the test buckets I named above.
I’ve enclosed our short form report. We break down the boards/arch combos for
each bucket pass/skip or potentially fails. Pretty straight forward. Skips
generally happen for a few reasons
1) crappy test cases
2) test isn’t appropriate (x86 specific tests so don’t run elsewhere)
With this, we have a decent baseline for 4.14 and other kernels going
forward.
Summary
------------------------------------------------------------------------
kernel: 4.14.0
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git branch: master
git commit: bebc6082da0a9f5d47a1ea2edc099bf671058bd4
git describe: v4.14
Test details: https://qa-reports.linaro.org/lkft/linux-mainline-oe/build/v4.14
No regressions (compared to build v4.14-rc8)
Boards, architectures and test suites:
-------------------------------------
hi6220-hikey - arm64
* boot - pass: 20
* kselftest - skip: 16, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 1, pass: 21
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 14
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 122, pass: 983
* ltp-timers-tests - pass: 12
juno-r2 - arm64
* boot - pass: 20
* kselftest - skip: 15, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 10
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 156, pass: 943
* ltp-timers-tests - pass: 12
x15 - arm
* boot - pass: 20
* kselftest - skip: 17, pass: 36
* libhugetlbfs - skip: 1, pass: 87
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 20
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 1, pass: 13
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 66, pass: 1040
* ltp-timers-tests - pass: 12
dell-poweredge-r200 - x86_64
* boot - pass: 19
* kselftest - skip: 11, pass: 54
* libhugetlbfs - skip: 1, pass: 76
* ltp-cap_bounds-tests - pass: 1
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 1, pass: 61
* ltp-fs_bind-tests - pass: 1
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 8
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 9
* ltp-securebits-tests - pass: 3
* ltp-syscalls-tests - skip: 163, pass: 962
Lots of green.
Let’s now talk about coverage, the pandora’s box of validation. It’s never
perfect. There’s a bazillion different build combos. Even tools can
make a difference. We’ve seen a case where the dhcp client from open embedded
didn’t trigger a network regression in one of the LTS RCs but Debian’s dhclient
did.
Of no surprise between what we and others have, it’s not perfect coverage,
and there are only so many build, boot and run cycles to execute the test
buckets with various combinations so we need to stay sensible as far as
kernel configs go.
Does this kind of system actually FIND anything and is it useful for
watching for 4.14 regressions as fixes are introduced?
I would assert the answer is yes. We do have data for a couple of kernel
cycles but it’s also somewhat dirty as we have been in the process of
detecting and tossing out dodgy test cases.
Take 4.14-RC7, there was one failure that is no longer there.
ltp-syscalls-tests : perf_event_open02 (arm64)
As things are getting merged post 4.14 there are some failures
cropping up. Here’s an example:
https://qa-reports.linaro.org/lkft/linux-mainline-oe/tests/ltp-fs-tests/pro…
Note the Build column, the kernels are identified by their git describe.
Don’t be alarmed if you see n/a in some columns, the queues are catching up
so data will be filling in.
So why didn’t we report these? As mentioned we’ve been tossing out dodgy
test cases to get to a clean baseline. We don’t need or want noise.
For LTS, I want the system when it detects a failure to enable a quick
bisect involving the affected test bucket. Given the nature of kernel
bugs tho, there is that class of bug which only happens occasionally.
This brings up a conundrum when you have a system like this. A failure
turns up, it’s not consistently failing and a path forward isn’t
necessarily obvious. Remember for an LTS RC, there’s a defined window
to comment.
I’ve been flamed for reporting a LTS RC test failure which didn't include
a fix, just a ‘this fails, and we’re looking at it.’ I’ve been flamed
for not reporting a failure that had been detected but not raised to the
list since it was still being debugged after the RC comment window had
closed.
My 1990s vintage asbestos underwear thankfully is functional.
There is probably a case to be made either way. It boils down to
either:
Red Pill) Be fully open reporting early and often
Blue Pill) Be closed and only pass up failures that include a patch to fix a bug.
Red Pill does expose drama yet it also creates an opportunity for others to
get involved.
Blue Pill protects the community from noise and the creation of frustration
that the system has cried wolf for perhaps a stupid test case.
Likewise from a maintainer or dev perspective, there’s a sea of data.
Time is precious, and who wants to waste it on some snipe hunt?
I’m personally in the Red Pill camp. I like being open.
Be it 0day, LKFT or whatever I think the responsibility is on us
running these projects to be open and give full guidance. Yes there
will be noise. Noise can suggest dodgy test cases or bugs that are
hard to trigger. Either way they warrant a look. Take Arnd Bergman’s
work to get rid of kernel warnings. Same concept in my opinion.
Dodgy test cases can easily be put onto skip lists. As we’ve been
running for a number of months now, data and ol fashioned code
review has been our guide to banish dodgy test cases to skip lists.
Going forward new test cases will pop up. Some of them will be dodgy.
There’s lots of room for collaboration in improving test cases.
In summary I think for mainline, LTS kernels etc, we have a good
warning system to detect regressions as patches flow in. It will evolve
and improve as is the nature of our open community. From kernelci,
LKFT, 0day, etc, that’s a good set of automated systems to ferret out
problems introduced by patches.
Tom--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 06, 2017 at 12:57:15AM -0800, Ram Pai wrote:
> PAPR defines 'ibm,processor-storage-keys' property. It exports two
> values. The first value holds the number of data-access keys and the
> second holds the number of instruction-access keys. Due to a bug in
> the firmware, instruction-access keys is always reported as zero.
> However any key can be configured to disable data-access and/or disable
> execution-access. The inavailablity of the second value is not a
> big handicap, though it could have been used to determine if the
> platform supported disable-execution-access.
>
> Non PAPR platforms do not define this property in the device tree yet.
> Here, we hardcode CPUs that support pkey by consulting
> PowerISA3.0
>
> This patch calculates the number of keys supported by the platform.
> Alsi it determines the platform support for read/write/execution access
> support for pkeys.
>
> Signed-off-by: Ram Pai <linuxram(a)us.ibm.com>
> ---
>
....snip...
> +static inline bool pkey_mmu_enabled(void)
> +{
> + if (firmware_has_feature(FW_FEATURE_LPAR))
> + return pkeys_total;
> + else
> + return cpu_has_feature(CPU_FTR_PKEY);
> +}
> +
> void __init pkey_initialize(void)
> {
> int os_reserved, i;
> @@ -46,14 +54,9 @@ void __init pkey_initialize(void)
> __builtin_popcountl(ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT)
> != (sizeof(u64) * BITS_PER_BYTE));
>
> - /*
> - * Disable the pkey system till everything is in place. A subsequent
> - * patch will enable it.
> - */
> - static_branch_enable(&pkey_disabled);
> -
> - /* Lets assume 32 keys */
> - pkeys_total = 32;
vvvvvvvvvvvvvvvvvvvv
> + /* Let's assume 32 keys if we are not told the number of pkeys. */
> + if (!pkeys_total)
> + pkeys_total = 32;
^^^^^^^^^^^^^^^^^^^^
There is a small bug here.
On a KVM guest or a LPAR, if the device tree
does not expose pkeys, the pkey-subsystem must be disabled.
Unfortunately, the code above blindly sets the pkeys_total to 32.
This confuses pkey_mmu_enabled() into returning true. Because of this
bug the guest errorneously enables pkey-subsystem.
The fix is to delete the code marked above.
>
> /*
> * Adjust the upper limit, based on the number of bits supported by
> @@ -62,11 +65,19 @@ void __init pkey_initialize(void)
> pkeys_total = min_t(int, pkeys_total,
> (ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT));
>
> + if (!pkey_mmu_enabled() || radix_enabled() || !pkeys_total)
> + static_branch_enable(&pkey_disabled);
> + else
> + static_branch_disable(&pkey_disabled);
> +
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi
Le 06/11/2017 à 09:56, Ram Pai a écrit :
> Memory protection keys enable applications to protect its
> address space from inadvertent access from or corruption
> by itself.
>
> These patches along with the pte-bit freeing patch series
> enables the protection key feature on powerpc; 4k and 64k
> hashpage kernels. It also changes the generic and x86
> code to expose memkey features through sysfs. Finally
> testcases and Documentation is updated.
>
> All patches can be found at --
> https://github.com/rampai/memorykeys.git memkey.v9
As far as I can see you are focussing the implementation on 64 bits
powerpc. This could also be implemented on 32 bits powerpc, for instance
the 8xx has MMU Access Protection Registers which can be used to define
16 domains and could I think be used for implementing protection keys.
Of course the challenge after that would be to find 4 spare PTE bits,
I'm sure we can find them on the 8xx, at least when using 16k pages we
have 2 bits already available, then by merging PAGE_SHARED and PAGE_USER
and by reducing PAGE_RO to only one bit we can get the 4 spare bits.
Therefore I think it would be great if you could implement a framework
common to both PPC32 and PPC64.
Christophe
>
> The overall idea:
> -----------------
> A process allocates a key and associates it with
> an address range within its address space.
> The process then can dynamically set read/write
> permissions on the key without involving the
> kernel. Any code that violates the permissions
> of the address space; as defined by its associated
> key, will receive a segmentation fault.
>
> This patch series enables the feature on PPC64 HPTE
> platform.
>
> ISA3.0 section 5.7.13 describes the detailed
> specifications.
>
>
> Highlevel view of the design:
> ---------------------------
> When an application associates a key with a address
> address range, program the key in the Linux PTE.
> When the MMU detects a page fault, allocate a hash
> page and program the key into HPTE. And finally
> when the MMU detects a key violation; due to
> invalid application access, invoke the registered
> signal handler and provide the violated key number.
>
>
> Testing:
> -------
> This patch series has passed all the protection key
> tests available in the selftest directory.The
> tests are updated to work on both x86 and powerpc.
> The selftests have passed on x86 and powerpc hardware.
>
> History:
> -------
> version v9:
> (1) used jump-labels to optimize code
> -- Balbir
> (2) fixed a register initialization bug noted
> by Balbir
> (3) fixed inappropriate use of paca to pass
> siginfo and keys to signal handler
> (4) Cleanup of comment style not to be right
> justified -- mpe
> (5) restructured the patches to depend on the
> availability of VM_PKEY_BIT4 in
> include/linux/mm.h
> (6) Incorporated comments from Dave Hansen
> towards changes to selftest and got
> them tested on x86.
>
> version v8:
> (1) Contents of the AMR register withdrawn from
> the siginfo structure. Applications can always
> read the AMR register.
> (2) AMR/IAMR/UAMOR are now available through
> ptrace system call. -- thanks to Thiago
> (3) code changes to handle legacy power cpus
> that do not support execute-disable.
> (4) incorporates many code improvement
> suggestions.
>
> version v7:
> (1) refers to device tree property to enable
> protection keys.
> (2) adds 4K PTE support.
> (3) fixes a couple of bugs noticed by Thiago
> (4) decouples this patch series from arch-
> independent code. This patch series can
> now stand by itself, with one kludge
> patch(2).
> version v7:
> (1) refers to device tree property to enable
> protection keys.
> (2) adds 4K PTE support.
> (3) fixes a couple of bugs noticed by Thiago
> (4) decouples this patch series from arch-
> independent code. This patch series can
> now stand by itself, with one kludge
> patch(2).
>
> version v6:
> (1) selftest changes are broken down into 20
> incremental patches.
> (2) A separate key allocation mask that
> includes PKEY_DISABLE_EXECUTE is
> added for powerpc
> (3) pkey feature is enabled for 64K HPT case
> only. RPT and 4k HPT is disabled.
> (4) Documentation is updated to better
> capture the semantics.
> (5) introduced arch_pkeys_enabled() to find
> if an arch enables pkeys. Correspond-
> ing change the logic that displays
> key value in smaps.
> (6) code rearranged in many places based on
> comments from Dave Hansen, Balbir,
> Anshuman.
> (7) fixed one bug where a bogus key could be
> associated successfully in
> pkey_mprotect().
>
> version v5:
> (1) reverted back to the old design -- store
> the key in the pte, instead of bypassing
> it. The v4 design slowed down the hash
> page path.
> (2) detects key violation when kernel is told
> to access user pages.
> (3) further refined the patches into smaller
> consumable units
> (4) page faults handlers captures the fault-
> ing key
> from the pte instead of the vma. This
> closes a race between where the key
> update in the vma and a key fault caused
> by the key programmed in the pte.
> (5) a key created with access-denied should
> also set it up to deny write. Fixed it.
> (6) protection-key number is displayed in
> smaps the x86 way.
>
> version v4:
> (1) patches no more depend on the pte bits
> to program the hpte
> -- comment by Balbir
> (2) documentation updates
> (3) fixed a bug in the selftest.
> (4) unlike x86, powerpc lets signal handler
> change key permission bits; the
> change will persist across signal
> handler boundaries. Earlier we
> allowed the signal handler to
> modify a field in the siginfo
> structure which would than be used
> by the kernel to program the key
> protection register (AMR)
> -- resolves a issue raised by Ben.
> "Calls to sys_swapcontext with a
> made-up context will end up with a
> crap AMR if done by code who didn't
> know about that register".
> (5) these changes enable protection keys on
> 4k-page kernel aswell.
>
> version v3:
> (1) split the patches into smaller consumable
> patches.
> (2) added the ability to disable execute
> permission on a key at creation.
> (3) rename calc_pte_to_hpte_pkey_bits() to
> pte_to_hpte_pkey_bits()
> -- suggested by Anshuman
> (4) some code optimization and clarity in
> do_page_fault()
> (5) A bug fix while invalidating a hpte slot
> in __hash_page_4K()
> -- noticed by Aneesh
>
>
> version v2:
> (1) documentation and selftest added.
> (2) fixed a bug in 4k hpte backed 64k pte
> where page invalidation was not
> done correctly, and initialization
> of second-part-of-the-pte was not
> done correctly if the pte was not
> yet Hashed with a hpte.
> -- Reported by Aneesh.
> (3) Fixed ABI breakage caused in siginfo
> structure.
> -- Reported by Anshuman.
>
>
> version v1: Initial version
>
> Ram Pai (47):
> mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS
> is enabled
> mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
> powerpc: initial pkey plumbing
> powerpc: track allocation status of all pkeys
> powerpc: helper function to read,write AMR,IAMR,UAMOR registers
> powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
> powerpc: cleanup AMR, IAMR when a key is allocated or freed
> powerpc: implementation for arch_set_user_pkey_access()
> powerpc: ability to create execute-disabled pkeys
> powerpc: store and restore the pkey state across context switches
> powerpc: introduce execute-only pkey
> powerpc: ability to associate pkey to a vma
> powerpc: implementation for arch_override_mprotect_pkey()
> powerpc: map vma key-protection bits to pte key bits.
> powerpc: Program HPTE key protection bits
> powerpc: helper to validate key-access permissions of a pte
> powerpc: check key protection for user page access
> powerpc: implementation for arch_vma_access_permitted()
> powerpc: Handle exceptions caused by pkey violation
> powerpc: introduce get_mm_addr_key() helper
> powerpc: Deliver SEGV signal on pkey violation
> powerpc: Enable pkey subsystem
> powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
> powerpc: sys_pkey_mprotect() system call
> powerpc: add sys_pkey_modify() system call
> mm, x86 : introduce arch_pkeys_enabled()
> mm: display pkey in smaps if arch_pkeys_enabled() is true
> Documentation/x86: Move protecton key documentation to arch neutral
> directory
> Documentation/vm: PowerPC specific updates to memory protection keys
> selftest/x86: Move protecton key selftest to arch neutral directory
> selftest/vm: rename all references to pkru to a generic name
> selftest/vm: move generic definitions to header file
> selftest/vm: typecast the pkey register
> selftest/vm: generic function to handle shadow key register
> selftest/vm: fix the wrong assert in pkey_disable_set()
> selftest/vm: fixed bugs in pkey_disable_clear()
> selftest/vm: clear the bits in shadow reg when a pkey is freed.
> selftest/vm: fix alloc_random_pkey() to make it really random
> selftest/vm: introduce two arch independent abstraction
> selftest/vm: pkey register should match shadow pkey
> selftest/vm: generic cleanup
> selftest/vm: powerpc implementation for generic abstraction
> selftest/vm: fix an assertion in test_pkey_alloc_exhaust()
> selftest/vm: associate key on a mapped page and detect access
> violation
> selftest/vm: associate key on a mapped page and detect write
> violation
> selftest/vm: detect write violation on a mapped access-denied-key
> page
> selftest/vm: sub-page allocator
>
> Thiago Jung Bauermann (4):
> powerpc/ptrace: Add memory protection key regset
> mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
> selftests/powerpc: Add ptrace tests for Protection Key register
> selftests/powerpc: Add core file test for Protection Key register
>
> Documentation/vm/protection-keys.txt | 161 +++
> Documentation/x86/protection-keys.txt | 85 --
> arch/powerpc/Kconfig | 15 +
> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +
> arch/powerpc/include/asm/book3s/64/mmu.h | 10 +
> arch/powerpc/include/asm/book3s/64/pgtable.h | 42 +-
> arch/powerpc/include/asm/bug.h | 1 +
> arch/powerpc/include/asm/cputable.h | 15 +-
> arch/powerpc/include/asm/mman.h | 13 +-
> arch/powerpc/include/asm/mmu.h | 9 +
> arch/powerpc/include/asm/mmu_context.h | 24 +
> arch/powerpc/include/asm/pkeys.h | 247 ++++
> arch/powerpc/include/asm/processor.h | 5 +
> arch/powerpc/include/asm/systbl.h | 4 +
> arch/powerpc/include/asm/unistd.h | 6 +-
> arch/powerpc/include/uapi/asm/elf.h | 1 +
> arch/powerpc/include/uapi/asm/mman.h | 6 +
> arch/powerpc/include/uapi/asm/unistd.h | 4 +
> arch/powerpc/kernel/entry_64.S | 9 +
> arch/powerpc/kernel/process.c | 7 +
> arch/powerpc/kernel/prom.c | 18 +
> arch/powerpc/kernel/ptrace.c | 66 +
> arch/powerpc/kernel/traps.c | 19 +-
> arch/powerpc/mm/Makefile | 1 +
> arch/powerpc/mm/fault.c | 49 +-
> arch/powerpc/mm/hash_utils_64.c | 29 +
> arch/powerpc/mm/mmu_context_book3s64.c | 2 +
> arch/powerpc/mm/pkeys.c | 463 +++++++
> arch/x86/include/asm/mmu_context.h | 4 +-
> arch/x86/include/asm/pkeys.h | 2 +
> arch/x86/kernel/fpu/xstate.c | 5 +
> arch/x86/kernel/setup.c | 8 -
> arch/x86/mm/pkeys.c | 9 +
> fs/proc/task_mmu.c | 16 +-
> include/linux/mm.h | 12 +-
> include/linux/pkeys.h | 7 +-
> include/uapi/linux/elf.h | 1 +
> mm/mprotect.c | 88 ++
> tools/testing/selftests/powerpc/include/reg.h | 1 +
> tools/testing/selftests/powerpc/ptrace/Makefile | 5 +-
> tools/testing/selftests/powerpc/ptrace/core-pkey.c | 438 ++++++
> .../testing/selftests/powerpc/ptrace/ptrace-pkey.c | 443 ++++++
> tools/testing/selftests/vm/Makefile | 1 +
> tools/testing/selftests/vm/pkey-helpers.h | 405 ++++++
> tools/testing/selftests/vm/protection_keys.c | 1464 ++++++++++++++++++++
> tools/testing/selftests/x86/Makefile | 2 +-
> tools/testing/selftests/x86/pkey-helpers.h | 220 ---
> tools/testing/selftests/x86/protection_keys.c | 1395 -------------------
> 48 files changed, 4095 insertions(+), 1747 deletions(-)
> create mode 100644 Documentation/vm/protection-keys.txt
> delete mode 100644 Documentation/x86/protection-keys.txt
> create mode 100644 arch/powerpc/include/asm/pkeys.h
> create mode 100644 arch/powerpc/mm/pkeys.c
> create mode 100644 tools/testing/selftests/powerpc/ptrace/core-pkey.c
> create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
> create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
> create mode 100644 tools/testing/selftests/vm/protection_keys.c
> delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
> delete mode 100644 tools/testing/selftests/x86/protection_keys.c
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ram,
On Mon, Nov 06, 2017 at 12:57:36AM -0800, Ram Pai wrote:
> @@ -206,12 +209,14 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
>
> trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
> ip = uctxt->uc_mcontext.gregs[REG_IP_IDX];
> - fpregset = uctxt->uc_mcontext.fpregs;
> - fpregs = (void *)fpregset;
Since you removed all references for fpregset now, you probably want to
remove the declaration of the variable above.
> @@ -219,20 +224,21 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
> * state. We just assume that it is here.
> */
> fpregs += 0x70;
> -#endif
> - pkey_reg_offset = pkey_reg_xstate_offset();
With this code, you removed all the reference for variable
pkey_reg_offset, thus, its declaration could be removed also.
> - *(u64 *)pkey_reg_ptr = 0x00000000;
> + dprintf1("si_pkey from siginfo: %lx\n", si_pkey);
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> + dprintf1("signal pkey_reg from xsave: %016lx\n", *pkey_reg_ptr);
> + *(u64 *)pkey_reg_ptr &= reset_bits(si_pkey, PKEY_DISABLE_ACCESS);
> +#elif __powerpc64__
Since the variable pkey_reg_ptr is only used for Intel code (inside
#ifdefs), you probably want to #ifdef the variable declaration also,
avoid triggering "unused variable" warning on non-Intel machines.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 06, 2017 at 05:22:18PM -0800, Ram Pai wrote:
> On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
> > * Ram Pai:
> >
> > > Testing:
> > > -------
> > > This patch series has passed all the protection key
> > > tests available in the selftest directory.The
> > > tests are updated to work on both x86 and powerpc.
> > > The selftests have passed on x86 and powerpc hardware.
> >
....snip....
> > What about siglongjmp from a signal handler?
>
> On powerpc there is some relief. the permissions on a key can be
> modified from anywhere, including from the signal handler, and the
> effect will be immediate. You dont have to wait till the
> signal handler returns for the key permissions to be restore.
>
> also after return from the sigsetjmp();
> possibly caused by siglongjmp(), the program can restore the permission
> on any key.
>
> Atleast that is my theory. Can you give me a testcase; if you have one
> handy.
>
> >
> > <https://sourceware.org/bugzilla/show_bug.cgi?id=22396>
> >
reading through the bug report, you mention that the following
"The application may not be able to save and restore the protection bits
for all keys because the kernel API does not actually specify that the
set of keys is a small, fixed set."
What exact kernel API do you need? This patch set exposes the total
number of keys and max keys, through sysfs.
https://marc.info/?l=linux-kernel&m=150995950219669&w=2
Is this sufficient? or do you need something else?
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/05/2017 03:56 AM, Lei Yang wrote:
> Replace '%d' by '%zu' to fix the following compilation warning.
>
> memfd_test.c:517:3: warning: format ‘%d’ expects argument of
> type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
> ^
> memfd_test.c: In function ‘mfd_fail_grow_write’:
> memfd_test.c:537:3: warning: format ‘%d’ expects argument
> of type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>
> Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
Thanks for the patch. Applied to linux-kselftest next for 4.15-rc1
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
At Linaro we run mainline/linux-next selftests on LTS releases and
run into few test failures due to kernel mismatch or missing upstream
functionality in older kernels. Discussed at length here:
https://lkml.org/lkml/2017/6/15/652
This patch series is an attempt to modify selftest firmware test scripts.
The proposed changes skip/ignore testing the upstream functionality
missing in the older kernel releases.
v2:
Changed the display message to make it consistent across all
the firmware test scripts. Added Fixes tag.
Regards,
Amit Pundir
Amit Pundir (2):
selftests: firmware: skip unsupported async loading tests
selftests: firmware: skip unsupported custom firmware fallback tests
tools/testing/selftests/firmware/fw_fallback.sh | 38 ++++++++++++++++-------
tools/testing/selftests/firmware/fw_filesystem.sh | 34 ++++++++++++--------
2 files changed, 47 insertions(+), 25 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
At Linaro we run mainline/linux-next selftests on LTS releases and
run into few test failures due to kernel mismatch or missing upstream
functionality in older kernels. Discussed at length here:
https://lkml.org/lkml/2017/6/15/652
This patch series is an attempt to modify selftest firmware test scripts.
The proposed changes skip/ignore testing the upstream functionality
missing in the older kernel releases.
Regards,
Amit Pundir
Amit Pundir (2):
selftests: firmware: skip unsupported async loading tests
selftests: firmware: skip unsupported custom firmware fallback tests
tools/testing/selftests/firmware/fw_fallback.sh | 38 ++++++++++++++++-------
tools/testing/selftests/firmware/fw_filesystem.sh | 34 ++++++++++++--------
2 files changed, 47 insertions(+), 25 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/06/2017 06:18 PM, lei yang wrote:
>
>
> On 2017年11月07日 07:48, Shuah Khan wrote:
>> On 11/06/2017 04:45 PM, Shuah Khan wrote:
>>> On 11/05/2017 09:03 PM, Lei Yang wrote:
>>>> I run into below error when building futext
>>>> /bin/sh: -c: line 5: syntax error: unexpected end of file
>>>>
>>>> the closing ";" and "\" are necessary.
>>>> My OS is "Ubuntu 14.04.5 LTS"
>>>>
>>>> Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
>>>> ---
>>>> tools/testing/selftests/futex/Makefile | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
>>>> index f0c0369..943f3a2 100644
>>>> --- a/tools/testing/selftests/futex/Makefile
>>>> +++ b/tools/testing/selftests/futex/Makefile
>>>> @@ -11,9 +11,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
>>>>
>>> Hmm. I don't see this error on linux_4.14-rc8?
>>>
>>> Can you give more details on the gcc version you are using?
>>
>> I meant gnumake version? I am not seeing the failure with what I am
>> using
>
> $ make -v
> GNU Make 3.81
> Copyright (C) 2006 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.
> There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
> PARTICULAR PURPOSE.
>
> This program built for x86_64-pc-linux-gnu
>
>
> Lei
>
I can't take patches that aren't tested on the latest kernel release.
3.16 is very old. I am not going reply to other patches you sent.
Please make sure the patches are based on the latest release and tested
on it as well.
thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
In next branch, Various build failed with:
In file included from membarrier_test.c:8:0:
../kselftest.h: In function ‘ksft_print_header’:
../kselftest.h:61:3: error: expected ‘)’ before ‘printf’
printf("TAP version 13\n");
^
../kselftest.h:62:1: error: expected expression before ‘}’ token
}
^
Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
---
tools/testing/selftests/kselftest.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index d6732ba312ef..acc98a5c3d73 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -57,7 +57,7 @@ static inline int ksft_get_error_cnt(void) { return ksft_cnt.ksft_error; }
static inline void ksft_print_header(void)
{
- if (!(getenv("KSFT_TAP_LEVEL"))
+ if (!(getenv("KSFT_TAP_LEVEL")))
printf("TAP version 13\n");
}
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I got the same warning even with ubuntu new distro, this patch fixed
this issue.
$ gcc --version
gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ make -v
GNU Make 4.1
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
$ uname -a
Linux pek-lyang0-u17 4.10.0-38-generic #42-Ubuntu SMP Tue Oct 10
13:24:27 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
---------------------
memfd_test.c: In function mfd_assert_grow_write:
memfd_test.c:517:19: warning: format %d expects argument of type int,
but argument 2 has type size_t {aka long unsigned int} [-Wformat=]
printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
^
memfd_test.c: In function mfd_fail_grow_write:
memfd_test.c:537:19: warning: format %d expects argument of type int,
but argument 2 has type size_t {aka long unsigned int} [-Wformat=]
printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
Lei
On 2017年11月07日 07:46, Shuah Khan wrote:
> On 11/05/2017 03:56 AM, Lei Yang wrote:
>> Replace '%d' by '%zu' to fix the following compilation warning.
>>
>> memfd_test.c:517:3: warning: format ‘%d’ expects argument of
>> type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
>> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>> ^
>> memfd_test.c: In function ‘mfd_fail_grow_write’:
>> memfd_test.c:537:3: warning: format ‘%d’ expects argument
>> of type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
>> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>>
>> Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
>> ---
>> tools/testing/selftests/memfd/memfd_test.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
>> index f94c6d1..95df9e6 100644
>> --- a/tools/testing/selftests/memfd/memfd_test.c
>> +++ b/tools/testing/selftests/memfd/memfd_test.c
>> @@ -514,7 +514,7 @@ static void mfd_assert_grow_write(int fd)
>>
>> buf = malloc(mfd_def_size * 8);
>> if (!buf) {
>> - printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>> + printf("malloc(%zu) failed: %m\n", mfd_def_size * 8);
>> abort();
>> }
>>
>> @@ -534,7 +534,7 @@ static void mfd_fail_grow_write(int fd)
>>
>> buf = malloc(mfd_def_size * 8);
>> if (!buf) {
>> - printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>> + printf("malloc(%zu) failed: %m\n", mfd_def_size * 8);
>> abort();
>> }
>>
>>
> Relates to gcc version perhaps. What's your gcc version?
>
> thanks,
> -- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/06/2017 06:14 PM, lei yang wrote:
>
>
> On 2017年11月07日 07:35, Shuah Khan wrote:
>> On 11/05/2017 03:08 AM, Lei Yang wrote:
>>> I got below error message when building sync test:
>>>
>>> make[1]: Entering directory `tools/testing/selftests/sync'
>>> gcc -c sync.c -o tools/testing/selftests/sync/sync.o
>>> sync.c:42:29: fatal error: linux/sync_file.h: No such file or directory
>>> #include <linux/sync_file.h>
>>>
>>> obviously, CFLAGS and LDFLAGS are not used when comipling.
>>>
>>> Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
>>> ---
>>> tools/testing/selftests/sync/Makefile | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
>>> index 8e04d0a..46cbcc3 100644
>>> --- a/tools/testing/selftests/sync/Makefile
>>> +++ b/tools/testing/selftests/sync/Makefile
>>> @@ -29,9 +29,9 @@ $(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) $(LDFLAGS)
>>> $(TESTS): $(OUTPUT)/%.o: %.c
>>> - $(CC) -c $^ -o $@
>>> + $(CC) -c $^ -o $@ $(CFLAGS) $(LDFLAGS)
>>> EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS)
>>>
>> How are you building the test? I am not seeing the error on linux-4.14.0-rc8
>
> make -C tools/testing/selftests
>
> Lei
>
>
Based on the information in your other emails, looks like you are
testing these patches on
$ cat /proc/version
Linux version 3.16.0-30-generic (buildd@kissel) (gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1) ) #40~14.04.1-Ubuntu SMP Thu Jan 15 17:43:14 UTC 2015
That explains why you aren't finding sync_file.h header. This test isn't applicable
to Linux 3.16. In any case, I can't take patches that aren't tested on the latest
kernel release. 3.16 is very old.
thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 07, 2017 at 08:32:16AM +0100, Florian Weimer wrote:
> * Ram Pai:
>
> > On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
> >> * Ram Pai:
> >>
> >> > Testing:
> >> > -------
> >> > This patch series has passed all the protection key
> >> > tests available in the selftest directory.The
> >> > tests are updated to work on both x86 and powerpc.
> >> > The selftests have passed on x86 and powerpc hardware.
> >>
> >> How do you deal with the key reuse problem? Is it the same as x86-64,
> >> where it's quite easy to accidentally grant existing threads access to
> >> a just-allocated key, either due to key reuse or a changed init_pkru
> >> parameter?
> >
> > I am not sure how on x86-64, two threads get allocated the same key
> > at the same time? the key allocation is guarded under the mmap_sem
> > semaphore. So there cannot be a race where two threads get allocated
> > the same key.
>
> The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
> sequence. The pthread_create call makes the new thread inherit the
> access rights of the current thread, but then the key is deallocated.
> Reallocation of the same key will have that thread retain its access
> rights, which is IMHO not correct.
(Dave Hansen: please correct me if I miss-speak below)
As per the current semantics of sys_pkey_free(); the way I understand it,
the calling thread is saying disassociate me from this key. Other
threads continue to be associated with the key and could continue to
get key-faults, but this calling thread will not get key-faults on that
key any more.
Also the key should not get reallocated till all the threads in the process
have disassocated from the key; by calling sys_pkey_free().
>From that point of view, I think there is a bug in the implementation of
pkey on x86 and now on powerpc aswell.
>
> > Can you point me to the issue, if it is already discussed somewhere?
>
> See ‘MPK: pkey_free and key reuse’ on various lists (including
> linux-mm and linux-arch).
>
> It has a test case attached which demonstrates the behavior.
>
> > As far as the semantics is concerned, a key allocated in one thread's
> > context has no meaning if used in some other threads context within the
> > same process. The app should not try to re-use a key allocated in a
> > thread's context in some other threads's context.
>
> Uh-oh, that's not how this feature works on x86-64 at all. There, the
> keys are a process-global resource. Treating them per-thread
> seriously reduces their usefulness.
Sorry. I was not thinking right. Let me restate.
A key is a global resource, but the permissions on a key is
local to a thread. For eg: the same key could disable
access on a page for one thread, while it could disable write
on the same page on another thread.
>
> >> What about siglongjmp from a signal handler?
> >
> > On powerpc there is some relief. the permissions on a key can be
> > modified from anywhere, including from the signal handler, and the
> > effect will be immediate. You dont have to wait till the
> > signal handler returns for the key permissions to be restore.
>
> My concern is that the signal handler knows nothing about protection
> keys, but the current x86-64 semantics will cause it to clobber the
> access rights of the current thread.
>
> > also after return from the sigsetjmp();
> > possibly caused by siglongjmp(), the program can restore the permission
> > on any key.
>
> So that's not really an option.
>
> > Atleast that is my theory. Can you give me a testcase; if you have one
> > handy.
>
> The glibc patch I posted under the ‘MPK: pkey_free and key reuse’
> thread covers this, too.
thanks. will try the test case with my kernel patches. But, on
powerpc one can change the permissions on the key in the signal handler
which takes into effect immediately, there should not be a bug
in powerpc.
x86 has this requirement where it has to return from the signal handler
back to the kernel in order to change the permission on a key,
it can cause issues with longjump.
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Naresh Kamboju <naresh.kamboju(a)linaro.org>
test_kmod.sh reported false failure when module not present.
Instead check test_bpf.ko is present in the path before loading it.
Signed-off-by: Naresh Kamboju <naresh.kamboju(a)linaro.org>
---
tools/testing/selftests/bpf/test_kmod.sh | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_kmod.sh b/tools/testing/selftests/bpf/test_kmod.sh
index 6d58cca8e235..2e5a1049e2f2 100755
--- a/tools/testing/selftests/bpf/test_kmod.sh
+++ b/tools/testing/selftests/bpf/test_kmod.sh
@@ -9,9 +9,11 @@ test_run()
echo "[ JIT enabled:$1 hardened:$2 ]"
dmesg -C
- insmod $SRC_TREE/lib/test_bpf.ko 2> /dev/null
- if [ $? -ne 0 ]; then
- rc=1
+ if [ -f ${SRC_TREE}/lib/test_bpf.ko ]; then
+ insmod ${SRC_TREE}/lib/test_bpf.ko 2> /dev/null
+ if [ $? -ne 0 ]; then
+ rc=1
+ fi
fi
rmmod test_bpf 2> /dev/null
dmesg | grep FAIL
--
2.14.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html