* [PATCH v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint @ 2018-11-21 19:21 Breno Leitao 2018-11-28 13:23 ` [PATCH] selftests/powerpc: New TM signal self test Breno Leitao 2018-12-23 13:27 ` [v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Michael Ellerman 0 siblings, 2 replies; 8+ messages in thread From: Breno Leitao @ 2018-11-21 19:21 UTC (permalink / raw) To: linuxppc-dev; +Cc: Breno Leitao, mikey, v3.9+, gromero On a signal handler return, the user could set a context with MSR[TS] bits set, and these bits would be copied to task regs->msr. At restore_tm_sigcontexts(), after current task regs->msr[TS] bits are set, several __get_user() are called and then a recheckpoint is executed. This is a problem since a page fault (in kernel space) could happen when calling __get_user(). If it happens, the process MSR[TS] bits were already set, but recheckpoint was not executed, and SPRs are still invalid. The page fault can cause the current process to be de-scheduled, with MSR[TS] active and without tm_recheckpoint() being called. More importantly, without TEXASR[FS] bit set also. Since TEXASR might not have the FS bit set, and when the process is scheduled back, it will try to reclaim, which will be aborted because of the CPU is not in the suspended state, and, then, recheckpoint. This recheckpoint will restore thread->texasr into TEXASR SPR, which might be zero, hitting a BUG_ON(). kernel BUG at /build/linux-sf3Co9/linux-4.9.30/arch/powerpc/kernel/tm.S:434! cpu 0xb: Vector: 700 (Program Check) at [c00000041f1576d0] pc: c000000000054550: restore_gprs+0xb0/0x180 lr: 0000000000000000 sp: c00000041f157950 msr: 8000000100021033 current = 0xc00000041f143000 paca = 0xc00000000fb86300 softe: 0 irq_happened: 0x01 pid = 1021, comm = kworker/11:1 kernel BUG at /build/linux-sf3Co9/linux-4.9.30/arch/powerpc/kernel/tm.S:434! Linux version 4.9.0-3-powerpc64le (debian-kernel@lists.debian.org) (gcc version 6.3.0 20170516 (Debian 6.3.0-18) ) #1 SMP Debian 4.9.30-2+deb9u2 (2017-06-26) enter ? for help [c00000041f157b30] c00000000001bc3c tm_recheckpoint.part.11+0x6c/0xa0 [c00000041f157b70] c00000000001d184 __switch_to+0x1e4/0x4c0 [c00000041f157bd0] c00000000082eeb8 __schedule+0x2f8/0x990 [c00000041f157cb0] c00000000082f598 schedule+0x48/0xc0 [c00000041f157ce0] c0000000000f0d28 worker_thread+0x148/0x610 [c00000041f157d80] c0000000000f96b0 kthread+0x120/0x140 [c00000041f157e30] c00000000000c0e0 ret_from_kernel_thread+0x5c/0x7c This patch simply delays the MSR[TS] set, so, if there is any page fault in the __get_user() section, it does not have regs->msr[TS] set, since the TM structures are still invalid, thus avoiding doing TM operations for in-kernel exceptions and possible process reschedule. With this patch, the MSR[TS] will only be set just before recheckpointing and setting TEXASR[FS] = 1, thus avoiding an interrupt with TM registers in invalid state. Other than that, if CONFIG_PREEMPT is set, there might be a preemption just after setting MSR[TS] and before tm_recheckpoint(), thus, this block must be atomic from a preemption perspective, thus, calling preempt_disable/enable() on this code. It is not possible to move tm_recheckpoint to happen earlier, because it is required to get the checkpointed registers from userspace, with __get_user(), thus, the only way to avoid this undesired behavior is delaying the MSR[TS] set. The 32-bits signal handler seems to be safe this current issue, but, it might be exposed to the preemption issue, thus, disabling preemption in this chunk of code. Changes from v2: * Run the critical section with preempt_disable. Fixes: 87b4e5393af7 ("powerpc/tm: Fix return of active 64bit signals") Cc: stable@vger.kernel.org (v3.9+) Signed-off-by: Breno Leitao <leitao@debian.org> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c index e6474a45cef5..fd59fef9931b 100644 --- a/arch/powerpc/kernel/signal_32.c +++ b/arch/powerpc/kernel/signal_32.c @@ -848,7 +848,23 @@ static long restore_tm_user_regs(struct pt_regs *regs, /* If TM bits are set to the reserved value, it's an invalid context */ if (MSR_TM_RESV(msr_hi)) return 1; - /* Pull in the MSR TM bits from the user context */ + + /* + * Disabling preemption, since it is unsafe to be preempted + * with MSR[TS] set without recheckpointing. + */ + preempt_disable(); + + /* + * CAUTION: + * After regs->MSR[TS] being updated, make sure that get_user(), + * put_user() or similar functions are *not* called. These + * functions can generate page faults which will cause the process + * to be de-scheduled with MSR[TS] set but without calling + * tm_recheckpoint(). This can cause a bug. + * + * Pull in the MSR TM bits from the user context + */ regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr_hi & MSR_TS_MASK); /* Now, recheckpoint. This loads up all of the checkpointed (older) * registers, including FP and V[S]Rs. After recheckpointing, the @@ -873,6 +889,8 @@ static long restore_tm_user_regs(struct pt_regs *regs, } #endif + preempt_enable(); + return 0; } #endif diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index 83d51bf586c7..bbd1c73243d7 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -467,20 +467,6 @@ static long restore_tm_sigcontexts(struct task_struct *tsk, if (MSR_TM_RESV(msr)) return -EINVAL; - /* pull in MSR TS bits from user context */ - regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr & MSR_TS_MASK); - - /* - * Ensure that TM is enabled in regs->msr before we leave the signal - * handler. It could be the case that (a) user disabled the TM bit - * through the manipulation of the MSR bits in uc_mcontext or (b) the - * TM bit was disabled because a sufficient number of context switches - * happened whilst in the signal handler and load_tm overflowed, - * disabling the TM bit. In either case we can end up with an illegal - * TM state leading to a TM Bad Thing when we return to userspace. - */ - regs->msr |= MSR_TM; - /* pull in MSR LE from user context */ regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE); @@ -572,6 +558,34 @@ static long restore_tm_sigcontexts(struct task_struct *tsk, tm_enable(); /* Make sure the transaction is marked as failed */ tsk->thread.tm_texasr |= TEXASR_FS; + + /* + * Disabling preemption, since it is unsafe to be preempted + * with MSR[TS] set without recheckpointing. + */ + preempt_disable(); + + /* pull in MSR TS bits from user context */ + regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr & MSR_TS_MASK); + + /* + * Ensure that TM is enabled in regs->msr before we leave the signal + * handler. It could be the case that (a) user disabled the TM bit + * through the manipulation of the MSR bits in uc_mcontext or (b) the + * TM bit was disabled because a sufficient number of context switches + * happened whilst in the signal handler and load_tm overflowed, + * disabling the TM bit. In either case we can end up with an illegal + * TM state leading to a TM Bad Thing when we return to userspace. + * + * CAUTION: + * After regs->MSR[TS] being updated, make sure that get_user(), + * put_user() or similar functions are *not* called. These + * functions can generate page faults which will cause the process + * to be de-scheduled with MSR[TS] set but without calling + * tm_recheckpoint(). This can cause a bug. + */ + regs->msr |= MSR_TM; + /* This loads the checkpointed FP/VEC state, if used */ tm_recheckpoint(&tsk->thread); @@ -585,6 +599,8 @@ static long restore_tm_sigcontexts(struct task_struct *tsk, regs->msr |= MSR_VEC; } + preempt_enable(); + return err; } #endif -- 2.19.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] selftests/powerpc: New TM signal self test 2018-11-21 19:21 [PATCH v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Breno Leitao @ 2018-11-28 13:23 ` Breno Leitao 2018-11-29 2:11 ` Michael Neuling 2018-12-20 12:51 ` Michael Ellerman 2018-12-23 13:27 ` [v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Michael Ellerman 1 sibling, 2 replies; 8+ messages in thread From: Breno Leitao @ 2018-11-28 13:23 UTC (permalink / raw) To: linuxppc-dev; +Cc: Breno Leitao, mikey, gromero A new self test that forces MSR[TS] to be set without calling any TM instruction. This test also tries to cause a page fault at a signal handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG when tm_recheckpoint() is called. This test is not deterministic since it is hard to guarantee that the page access will cause a page fault. Tests have shown that the bug could be exposed with few interactions in a buggy kernel. This test is configured to loop 5000x, having a good chance to hit the kernel issue in just one run. This self test takes less than two seconds to run. This test uses set/getcontext because the kernel will recheckpoint zeroed structures, causing the test to segfault, which is undesired because the test needs to rerun, so, there is a signal handler for SIGSEGV which will restart the test. Signed-off-by: Breno Leitao <leitao@debian.org> --- tools/testing/selftests/powerpc/tm/.gitignore | 1 + tools/testing/selftests/powerpc/tm/Makefile | 3 +- .../powerpc/tm/tm-signal-force-msr.c | 115 ++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore index c3ee8393dae8..89679822ebc9 100644 --- a/tools/testing/selftests/powerpc/tm/.gitignore +++ b/tools/testing/selftests/powerpc/tm/.gitignore @@ -11,6 +11,7 @@ tm-signal-context-chk-fpu tm-signal-context-chk-gpr tm-signal-context-chk-vmx tm-signal-context-chk-vsx +tm-signal-force-msr tm-vmx-unavail tm-unavailable tm-trap diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile index 9fc2cf6fbc92..58a2ebd13958 100644 --- a/tools/testing/selftests/powerpc/tm/Makefile +++ b/tools/testing/selftests/powerpc/tm/Makefile @@ -4,7 +4,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \ tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap \ - $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn + $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-force-msr top_srcdir = ../../../../.. include ../../lib.mk @@ -20,6 +20,7 @@ $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64 $(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 +$(OUTPUT)/tm-signal-force-msr: CFLAGS += -pthread SIGNAL_CONTEXT_CHK_TESTS := $(patsubst %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS)) $(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c new file mode 100644 index 000000000000..4441d61c2328 --- /dev/null +++ b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2018, Breno Leitao, Gustavo Romero, IBM Corp. + */ + +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <signal.h> +#include <string.h> +#include <ucontext.h> +#include <unistd.h> + +#include "tm.h" +#include "utils.h" + +#define __MASK(X) (1UL<<(X)) +#define MSR_TS_S_LG 33 /* Trans Mem state: Suspended */ +#define MSR_TM __MASK(MSR_TM_LG) /* Transactional Mem Available */ +#define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */ + +#define COUNT_MAX 5000 /* Number of interactions */ + +/* Setting contexts because the test will crash and we want to recover */ +ucontext_t init_context, main_context; + +static int count, first_time; + +void trap_signal_handler(int signo, siginfo_t *si, void *uc) +{ + ucontext_t *ucp = uc; + + /* + * Allocating memory in a signal handler, and never freeing it on + * purpose, forcing the heap increase, so, the memory leak is what + * we want here. + */ + ucp->uc_link = malloc(sizeof(ucontext_t)); + memcpy(&ucp->uc_link, &ucp->uc_mcontext, sizeof(ucp->uc_mcontext)); + + /* Forcing to enable MSR[TM] */ + ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; + + /* + * A fork inside a signal handler seems to be more efficient than a + * fork() prior to the signal being raised. + */ + if (fork() == 0) { + /* + * Both child and parent will return, but, child returns + * with count set so it will exit in the next segfault. + * Parent will continue to loop. + */ + count = COUNT_MAX; + } + + /* + * If the change above does not hit the bug, it will cause a + * segmentation fault, since the ck structures are NULL. + */ +} + +void seg_signal_handler(int signo, siginfo_t *si, void *uc) +{ + if (count == COUNT_MAX) { + /* Return to tm_signal_force_msr() and exit */ + setcontext(&main_context); + } + + count++; + /* Reexecute the test */ + setcontext(&init_context); +} + +void tm_trap_test(void) +{ + struct sigaction trap_sa, seg_sa; + + trap_sa.sa_flags = SA_SIGINFO; + trap_sa.sa_sigaction = trap_signal_handler; + + seg_sa.sa_flags = SA_SIGINFO; + seg_sa.sa_sigaction = seg_signal_handler; + + /* + * Set initial context. Will get back here from + * seg_signal_handler() + */ + getcontext(&init_context); + + /* The signal handler will enable MSR_TS */ + sigaction(SIGUSR1, &trap_sa, NULL); + /* If it does not crash, it will segfault, avoid it to retest */ + sigaction(SIGSEGV, &seg_sa, NULL); + + raise(SIGUSR1); +} + +int tm_signal_force_msr(void) +{ + SKIP_IF(!have_htm()); + + /* Will get back here after COUNT_MAX interactions */ + getcontext(&main_context); + + if (!first_time++) + tm_trap_test(); + + return EXIT_SUCCESS; +} + +int main(int argc, char **argv) +{ + test_harness(tm_signal_force_msr, "tm_signal_force_msr"); +} -- 2.19.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/powerpc: New TM signal self test 2018-11-28 13:23 ` [PATCH] selftests/powerpc: New TM signal self test Breno Leitao @ 2018-11-29 2:11 ` Michael Neuling 2018-12-04 17:51 ` Breno Leitao 2018-12-20 12:51 ` Michael Ellerman 1 sibling, 1 reply; 8+ messages in thread From: Michael Neuling @ 2018-11-29 2:11 UTC (permalink / raw) To: Breno Leitao, linuxppc-dev; +Cc: gromero On Wed, 2018-11-28 at 11:23 -0200, Breno Leitao wrote: > A new self test that forces MSR[TS] to be set without calling any TM > instruction. This test also tries to cause a page fault at a signal > handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing > thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG > when tm_recheckpoint() is called. > > This test is not deterministic since it is hard to guarantee that the page > access will cause a page fault. Tests have shown that the bug could be > exposed with few interactions in a buggy kernel. This test is configured to > loop 5000x, having a good chance to hit the kernel issue in just one run. > This self test takes less than two seconds to run. You could try using sigaltstack() to put the ucontext somewhere else. Then you could play tricks with that memory to try to force a fault. madvise()+MADV_DONTNEED or fadvise()+POSIX_FADV_DONTNEED might do the trick. This is more extra credit to make it more reliable. Not a requirement. > This test uses set/getcontext because the kernel will recheckpoint > zeroed structures, causing the test to segfault, which is undesired because > the test needs to rerun, so, there is a signal handler for SIGSEGV which > will restart the test. Please put this description at the top of the test also. Other than that, it looks good. Mikey > > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > tools/testing/selftests/powerpc/tm/.gitignore | 1 + > tools/testing/selftests/powerpc/tm/Makefile | 3 +- > .../powerpc/tm/tm-signal-force-msr.c | 115 ++++++++++++++++++ > 3 files changed, 118 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c > > diff --git a/tools/testing/selftests/powerpc/tm/.gitignore > b/tools/testing/selftests/powerpc/tm/.gitignore > index c3ee8393dae8..89679822ebc9 100644 > --- a/tools/testing/selftests/powerpc/tm/.gitignore > +++ b/tools/testing/selftests/powerpc/tm/.gitignore > @@ -11,6 +11,7 @@ tm-signal-context-chk-fpu > tm-signal-context-chk-gpr > tm-signal-context-chk-vmx > tm-signal-context-chk-vsx > +tm-signal-force-msr > tm-vmx-unavail > tm-unavailable > tm-trap > diff --git a/tools/testing/selftests/powerpc/tm/Makefile > b/tools/testing/selftests/powerpc/tm/Makefile > index 9fc2cf6fbc92..58a2ebd13958 100644 > --- a/tools/testing/selftests/powerpc/tm/Makefile > +++ b/tools/testing/selftests/powerpc/tm/Makefile > @@ -4,7 +4,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm- > signal-context-chk-fpu > > TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal- > stack \ > tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap > \ > - $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn > + $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-force-msr > > top_srcdir = ../../../../.. > include ../../lib.mk > @@ -20,6 +20,7 @@ $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64 > $(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 > +$(OUTPUT)/tm-signal-force-msr: CFLAGS += -pthread > > SIGNAL_CONTEXT_CHK_TESTS := $(patsubst > %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS)) > $(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S > diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c > b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c > new file mode 100644 > index 000000000000..4441d61c2328 > --- /dev/null > +++ b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c > @@ -0,0 +1,115 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2018, Breno Leitao, Gustavo Romero, IBM Corp. > + */ > + > +#define _GNU_SOURCE > +#include <stdio.h> > +#include <stdlib.h> > +#include <signal.h> > +#include <string.h> > +#include <ucontext.h> > +#include <unistd.h> > + > +#include "tm.h" > +#include "utils.h" > + > +#define __MASK(X) (1UL<<(X)) > +#define MSR_TS_S_LG 33 /* Trans Mem state: Suspended */ > +#define MSR_TM __MASK(MSR_TM_LG) /* Transactional Mem Available */ > +#define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */ Surely we have these defined somewhere else in selftests? > + > +#define COUNT_MAX 5000 /* Number of interactions */ > + > +/* Setting contexts because the test will crash and we want to recover */ > +ucontext_t init_context, main_context; > + > +static int count, first_time; > + > +void trap_signal_handler(int signo, siginfo_t *si, void *uc) > +{ > + ucontext_t *ucp = uc; > + > + /* > + * Allocating memory in a signal handler, and never freeing it on > + * purpose, forcing the heap increase, so, the memory leak is what > + * we want here. > + */ > + ucp->uc_link = malloc(sizeof(ucontext_t)); > + memcpy(&ucp->uc_link, &ucp->uc_mcontext, sizeof(ucp->uc_mcontext)); > + > + /* Forcing to enable MSR[TM] */ > + ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; > + > + /* > + * A fork inside a signal handler seems to be more efficient than a > + * fork() prior to the signal being raised. > + */ > + if (fork() == 0) { > + /* > + * Both child and parent will return, but, child returns > + * with count set so it will exit in the next segfault. > + * Parent will continue to loop. > + */ > + count = COUNT_MAX; > + } > + > + /* > + * If the change above does not hit the bug, it will cause a > + * segmentation fault, since the ck structures are NULL. > + */ > +} > + > +void seg_signal_handler(int signo, siginfo_t *si, void *uc) > +{ > + if (count == COUNT_MAX) { > + /* Return to tm_signal_force_msr() and exit */ > + setcontext(&main_context); > + } > + > + count++; > + /* Reexecute the test */ > + setcontext(&init_context); > +} > + > +void tm_trap_test(void) > +{ > + struct sigaction trap_sa, seg_sa; > + > + trap_sa.sa_flags = SA_SIGINFO; > + trap_sa.sa_sigaction = trap_signal_handler; > + > + seg_sa.sa_flags = SA_SIGINFO; > + seg_sa.sa_sigaction = seg_signal_handler; > + > + /* > + * Set initial context. Will get back here from > + * seg_signal_handler() > + */ > + getcontext(&init_context); > + > + /* The signal handler will enable MSR_TS */ > + sigaction(SIGUSR1, &trap_sa, NULL); > + /* If it does not crash, it will segfault, avoid it to retest */ > + sigaction(SIGSEGV, &seg_sa, NULL); > + > + raise(SIGUSR1); > +} > + > +int tm_signal_force_msr(void) > +{ > + SKIP_IF(!have_htm()); > + > + /* Will get back here after COUNT_MAX interactions */ > + getcontext(&main_context); > + > + if (!first_time++) > + tm_trap_test(); > + > + return EXIT_SUCCESS; > +} > + > +int main(int argc, char **argv) > +{ > + test_harness(tm_signal_force_msr, "tm_signal_force_msr"); > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/powerpc: New TM signal self test 2018-11-29 2:11 ` Michael Neuling @ 2018-12-04 17:51 ` Breno Leitao 0 siblings, 0 replies; 8+ messages in thread From: Breno Leitao @ 2018-12-04 17:51 UTC (permalink / raw) To: Michael Neuling, linuxppc-dev; +Cc: gromero Hi Mikey, On 11/29/18 12:11 AM, Michael Neuling wrote: > On Wed, 2018-11-28 at 11:23 -0200, Breno Leitao wrote: >> A new self test that forces MSR[TS] to be set without calling any TM >> instruction. This test also tries to cause a page fault at a signal >> handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing >> thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG >> when tm_recheckpoint() is called. >> >> This test is not deterministic since it is hard to guarantee that the page >> access will cause a page fault. Tests have shown that the bug could be >> exposed with few interactions in a buggy kernel. This test is configured to >> loop 5000x, having a good chance to hit the kernel issue in just one run. >> This self test takes less than two seconds to run. > > You could try using sigaltstack() to put the ucontext somewhere else. Then you > could play tricks with that memory to try to force a fault. > madvise()+MADV_DONTNEED or fadvise()+POSIX_FADV_DONTNEED might do the trick. Yes, it sounded interesting and I implemented the test using madvice(). Thanks for the suggestion! The current approach didn't seem to improve the amount of page faults at it seems that MADV_DONTNEED makes no difference when using a Lazy page loading. This is the test I did, where 'original' is my current patch and 'madvice` is the patch below: Performance counter stats for './original': 0 major-faults 125,100 minor-faults 2.575479619 seconds time elapsed Performance counter stats for './madvice': 0 major-faults 125,099 minor-faults Other than that, I didn't see any improvements in the reproduction rate also, although it is a bit challenging to measure, since it crashes the machine and I can't run a full statistical model. This is the current patch I compared to the original one --- commit 082a9fe29412943adfa2d6a363f44bac8e81d0ce Author: Breno Leitao <leitao@debian.org> Date: Tue Nov 13 18:02:57 2018 -0500 selftests/powerpc: New TM signal self test A new self test that forces MSR[TS] to be set without calling any TM instruction. This test also tries to cause a page fault at a signal handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG when tm_recheckpoint() is called. This test is not deterministic, since it is hard to guarantee that the page access will cause a page fault. In order to force more page faults at signal context, the signal handler and the ucontext are being mapped into a MADV_DONTNEED memory chunks. Tests have shown that the bug could be exposed with few interactions in a buggy kernel. This test is configured to loop 5000x, having a good chance to hit the kernel issue in just one run. This self test takes less than two seconds to run. This test uses set/getcontext because the kernel will recheckpoint zeroed structures, causing the test to segfault, which is undesired because the test needs to rerun, so, there is a signal handler for SIGSEGV which will restart the test. Signed-off-by: Breno Leitao <leitao@debian.org> diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore index c3ee8393dae8..89679822ebc9 100644 --- a/tools/testing/selftests/powerpc/tm/.gitignore +++ b/tools/testing/selftests/powerpc/tm/.gitignore @@ -11,6 +11,7 @@ tm-signal-context-chk-fpu tm-signal-context-chk-gpr tm-signal-context-chk-vmx tm-signal-context-chk-vsx +tm-signal-force-msr tm-vmx-unavail tm-unavailable tm-trap diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile index 9fc2cf6fbc92..58a2ebd13958 100644 --- a/tools/testing/selftests/powerpc/tm/Makefile +++ b/tools/testing/selftests/powerpc/tm/Makefile @@ -4,7 +4,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \ tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap \ - $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn + $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-force-msr top_srcdir = ../../../../.. include ../../lib.mk @@ -20,6 +20,7 @@ $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64 $(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 +$(OUTPUT)/tm-signal-force-msr: CFLAGS += -pthread SIGNAL_CONTEXT_CHK_TESTS := $(patsubst %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS)) $(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c new file mode 100644 index 000000000000..496596f3c1bf --- /dev/null +++ b/tools/testing/selftests/powerpc/tm/tm-signal-force-msr.c @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2018, Breno Leitao, Gustavo Romero, IBM Corp. + * + * This test raises a SIGUSR1 signal, and toggle the MSR[TS] + * fields at the signal handler. With MSR[TS] being set, the kernel will + * force a recheckpoint, which may cause a segfault when returning to + * user space. Since the kernel needs to re-run, the segfault needs to be + * caught and handled. + * + * In order to continue the test even after a segfault, the context is + * saved prior to the signal being raised, and it is restored when there is + * a segmentation fault. This happens for COUNT_MAX times. + */ + +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <signal.h> +#include <string.h> +#include <ucontext.h> +#include <unistd.h> +#include <sys/mman.h> + +#include "tm.h" +#include "utils.h" +#include "reg.h" + +#define COUNT_MAX 5000 /* Number of interactions */ + +/* Setting contexts because the test will crash and we want to recover */ +ucontext_t init_context, main_context; + +static int count, first_time; + +void usr_signal_handler(int signo, siginfo_t *si, void *uc) +{ + ucontext_t *ucp = uc; + int ret; + + /* + * Allocating memory in a signal handler, and never freeing it on + * purpose, forcing the heap increase, so, the memory leak is what + * we want here. + */ + ucp->uc_link = mmap(NULL, sizeof(ucontext_t), + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); + if (ucp->uc_link == (void *)-1) { + perror("Mmap failed"); + exit(-1); + } + + /* Forcing the page to be allocated in a page fault */ + ret = madvise(ucp->uc_link, sizeof(ucontext_t), MADV_DONTNEED); + if (ret) { + perror("madvise failed"); + exit(-1); + } + + memcpy(&ucp->uc_link, &ucp->uc_mcontext, sizeof(ucp->uc_mcontext)); + + /* Forcing to enable MSR[TM] */ + ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; + + /* + * A fork inside a signal handler seems to be more efficient than a + * fork() prior to the signal being raised. + */ + if (fork() == 0) { + /* + * Both child and parent will return, but, child returns + * with count set so it will exit in the next segfault. + * Parent will continue to loop. + */ + count = COUNT_MAX; + } + + /* + * If the change above does not hit the bug, it will cause a + * segmentation fault, since the ck structures are NULL. + */ +} + +void seg_signal_handler(int signo, siginfo_t *si, void *uc) +{ + if (count == COUNT_MAX) { + /* Return to tm_signal_force_msr() and exit */ + setcontext(&main_context); + } + + count++; + + /* Reexecute the test */ + setcontext(&init_context); +} + +void tm_trap_test(void) +{ + struct sigaction usr_sa, seg_sa; + stack_t ss; + + usr_sa.sa_flags = SA_SIGINFO | SA_ONSTACK; + usr_sa.sa_sigaction = usr_signal_handler; + + seg_sa.sa_flags = SA_SIGINFO; + seg_sa.sa_sigaction = seg_signal_handler; + + /* + * Set initial context. Will get back here from + * seg_signal_handler() + */ + getcontext(&init_context); + + /* Allocated am alternative signal stack area */ + ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); + ss.ss_size = SIGSTKSZ; + ss.ss_flags = 0; + + if (ss.ss_sp == (void *)-1) { + perror("mmap error\n"); + exit(-1); + } + + /* Force the allocation through a page fault */ + if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) { + perror("madvise\n"); + exit(-1); + } + + /* Setting a alternative stack to generate a page fault when + * the signal is raised. + */ + if (sigaltstack(&ss, NULL)) { + perror("sigaltstack\n"); + exit(-1); + } + + /* The signal handler will enable MSR_TS */ + sigaction(SIGUSR1, &usr_sa, NULL); + /* If it does not crash, it will segfault, avoid it to retest */ + sigaction(SIGSEGV, &seg_sa, NULL); + + raise(SIGUSR1); +} + +int tm_signal_force_msr(void) +{ + SKIP_IF(!have_htm()); + + /* Will get back here after COUNT_MAX interactions */ + getcontext(&main_context); + + if (!first_time++) + tm_trap_test(); + + return EXIT_SUCCESS; +} + +int main(int argc, char **argv) +{ + test_harness(tm_signal_force_msr, "tm_signal_force_msr"); +} ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/powerpc: New TM signal self test 2018-11-28 13:23 ` [PATCH] selftests/powerpc: New TM signal self test Breno Leitao 2018-11-29 2:11 ` Michael Neuling @ 2018-12-20 12:51 ` Michael Ellerman 2019-01-03 13:05 ` Breno Leitao 1 sibling, 1 reply; 8+ messages in thread From: Michael Ellerman @ 2018-12-20 12:51 UTC (permalink / raw) To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, mikey, gromero Breno Leitao <leitao@debian.org> writes: > A new self test that forces MSR[TS] to be set without calling any TM > instruction. This test also tries to cause a page fault at a signal > handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing > thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG > when tm_recheckpoint() is called. > > This test is not deterministic since it is hard to guarantee that the page > access will cause a page fault. Tests have shown that the bug could be > exposed with few interactions in a buggy kernel. This test is configured to > loop 5000x, having a good chance to hit the kernel issue in just one run. > This self test takes less than two seconds to run. > > This test uses set/getcontext because the kernel will recheckpoint > zeroed structures, causing the test to segfault, which is undesired because > the test needs to rerun, so, there is a signal handler for SIGSEGV which > will restart the test. Hi Breno, Thanks for the test, some of these TM tests are getting pretty advanced! :) Unfortunately it doesn't build in a few configurations. On Ubuntu 18.10 built with powerpc-linux-gnu-gcc I get: tm-signal-force-msr.c: In function 'trap_signal_handler': tm-signal-force-msr.c:42:19: error: 'union uc_regs_ptr' has no member named 'gp_regs'; did you mean 'uc_regs'? ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; ^~~~~~~ uc_regs tm-signal-force-msr.c:17:29: error: left shift count >= width of type [-Werror=shift-count-overflow] #define __MASK(X) (1UL<<(X)) ^~ tm-signal-force-msr.c:20:25: note: in expansion of macro '__MASK' #define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */ ^~~~~~ tm-signal-force-msr.c:42:38: note: in expansion of macro 'MSR_TS_S' ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; ^~~~~~~~ And using powerpc64le-linux-gnu-gcc I get: In file included from /usr/powerpc64le-linux-gnu/include/string.h:494, from tm-signal-force-msr.c:10: In function 'memcpy', inlined from 'trap_signal_handler' at tm-signal-force-msr.c:39:2: /usr/powerpc64le-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin_memcpy' accessing 1272 bytes at offsets 8 and 168 overlaps 1112 bytes at offset 168 [-Werror=restrict] return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/powerpc: New TM signal self test 2018-12-20 12:51 ` Michael Ellerman @ 2019-01-03 13:05 ` Breno Leitao 2019-01-08 10:16 ` Michael Ellerman 0 siblings, 1 reply; 8+ messages in thread From: Breno Leitao @ 2019-01-03 13:05 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: mikey, gromero Hi Michael, On 12/20/18 10:51 AM, Michael Ellerman wrote: > Breno Leitao <leitao@debian.org> writes: > >> A new self test that forces MSR[TS] to be set without calling any TM >> instruction. This test also tries to cause a page fault at a signal >> handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing >> thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG >> when tm_recheckpoint() is called. >> >> This test is not deterministic since it is hard to guarantee that the page >> access will cause a page fault. Tests have shown that the bug could be >> exposed with few interactions in a buggy kernel. This test is configured to >> loop 5000x, having a good chance to hit the kernel issue in just one run. >> This self test takes less than two seconds to run. >> >> This test uses set/getcontext because the kernel will recheckpoint >> zeroed structures, causing the test to segfault, which is undesired because >> the test needs to rerun, so, there is a signal handler for SIGSEGV which >> will restart the test. > And reference the ucontext->mcontext MSR using UCONTEXT_MSR() macro. > Hi Breno, > > Thanks for the test, some of these TM tests are getting pretty advanced! :) > > Unfortunately it doesn't build in a few configurations. > > On Ubuntu 18.10 built with powerpc-linux-gnu-gcc I get: > > tm-signal-force-msr.c: In function 'trap_signal_handler': > tm-signal-force-msr.c:42:19: error: 'union uc_regs_ptr' has no member named 'gp_regs'; did you mean 'uc_regs'? > ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; > ^~~~~~~ > uc_regs > tm-signal-force-msr.c:17:29: error: left shift count >= width of type [-Werror=shift-count-overflow] > #define __MASK(X) (1UL<<(X)) > ^~ > tm-signal-force-msr.c:20:25: note: in expansion of macro '__MASK' > #define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */ > ^~~~~~ > tm-signal-force-msr.c:42:38: note: in expansion of macro 'MSR_TS_S' > ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; > ^~~~~~~~ > That is because I missed the -m64 compilation flag on Makefile. I understand that this test only make sense when compiled in 64 bits. Do you agree? I might also add a macro to address ucontext->mcontext MSR. This will avoid problems like that in the future. index ae43a614835d..7636bf45d5d5 100644 --- a/tools/testing/selftests/powerpc/include/utils.h +++ b/tools/testing/selftests/powerpc/include/utils.h @@ -102,8 +102,10 @@ do { #if defined(__powerpc64__) #define UCONTEXT_NIA(UC) (UC)->uc_mcontext.gp_regs[PT_NIP] +#define UCONTEXT_MSR(UC) (UC)->uc_mcontext.gp_regs[PT_MSR] #elif defined(__powerpc__) #define UCONTEXT_NIA(UC) (UC)->uc_mcontext.uc_regs->gregs[PT_NIP] +#define UCONTEXT_MSR(UC) (UC)->uc_mcontext.uc_regs->gregs[PT_MSR] #else #error implement UCONTEXT_NIA #endif > And using powerpc64le-linux-gnu-gcc I get: > > In file included from /usr/powerpc64le-linux-gnu/include/string.h:494, > from tm-signal-force-msr.c:10: > In function 'memcpy', > inlined from 'trap_signal_handler' at tm-signal-force-msr.c:39:2: > /usr/powerpc64le-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin_memcpy' accessing 1272 bytes at offsets 8 and 168 overlaps 1112 bytes at offset 168 [-Werror=restrict] > return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest)); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Damn, that is because I do not know how to use C pointers. Fixing it on v3 also. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] selftests/powerpc: New TM signal self test 2019-01-03 13:05 ` Breno Leitao @ 2019-01-08 10:16 ` Michael Ellerman 0 siblings, 0 replies; 8+ messages in thread From: Michael Ellerman @ 2019-01-08 10:16 UTC (permalink / raw) To: Breno Leitao, linuxppc-dev; +Cc: mikey, gromero Breno Leitao <leitao@debian.org> writes: > On 12/20/18 10:51 AM, Michael Ellerman wrote: >> Breno Leitao <leitao@debian.org> writes: >> >>> A new self test that forces MSR[TS] to be set without calling any TM >>> instruction. This test also tries to cause a page fault at a signal >>> handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing >>> thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG >>> when tm_recheckpoint() is called. >>> >>> This test is not deterministic since it is hard to guarantee that the page >>> access will cause a page fault. Tests have shown that the bug could be >>> exposed with few interactions in a buggy kernel. This test is configured to >>> loop 5000x, having a good chance to hit the kernel issue in just one run. >>> This self test takes less than two seconds to run. >>> >>> This test uses set/getcontext because the kernel will recheckpoint >>> zeroed structures, causing the test to segfault, which is undesired because >>> the test needs to rerun, so, there is a signal handler for SIGSEGV which >>> will restart the test. >> And reference the ucontext->mcontext MSR using UCONTEXT_MSR() macro. >> Hi Breno, >> >> Thanks for the test, some of these TM tests are getting pretty advanced! :) >> >> Unfortunately it doesn't build in a few configurations. >> >> On Ubuntu 18.10 built with powerpc-linux-gnu-gcc I get: >> >> tm-signal-force-msr.c: In function 'trap_signal_handler': >> tm-signal-force-msr.c:42:19: error: 'union uc_regs_ptr' has no member named 'gp_regs'; did you mean 'uc_regs'? >> ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; >> ^~~~~~~ >> uc_regs >> tm-signal-force-msr.c:17:29: error: left shift count >= width of type [-Werror=shift-count-overflow] >> #define __MASK(X) (1UL<<(X)) >> ^~ >> tm-signal-force-msr.c:20:25: note: in expansion of macro '__MASK' >> #define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */ >> ^~~~~~ >> tm-signal-force-msr.c:42:38: note: in expansion of macro 'MSR_TS_S' >> ucp->uc_mcontext.gp_regs[PT_MSR] |= MSR_TS_S; >> ^~~~~~~~ >> > > That is because I missed the -m64 compilation flag on Makefile. I understand > that this test only make sense when compiled in 64 bits. Do you agree? I think the test could work as a 32-bit binary on a 64-bit kernel, but I don't mind if you force it to build 64-bit. cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint 2018-11-21 19:21 [PATCH v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Breno Leitao 2018-11-28 13:23 ` [PATCH] selftests/powerpc: New TM signal self test Breno Leitao @ 2018-12-23 13:27 ` Michael Ellerman 1 sibling, 0 replies; 8+ messages in thread From: Michael Ellerman @ 2018-12-23 13:27 UTC (permalink / raw) To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, mikey, v3.9+, gromero On Wed, 2018-11-21 at 19:21:09 UTC, Breno Leitao wrote: > On a signal handler return, the user could set a context with MSR[TS] bits > set, and these bits would be copied to task regs->msr. > > At restore_tm_sigcontexts(), after current task regs->msr[TS] bits are set, > several __get_user() are called and then a recheckpoint is executed. > > This is a problem since a page fault (in kernel space) could happen when > calling __get_user(). If it happens, the process MSR[TS] bits were > already set, but recheckpoint was not executed, and SPRs are still invalid. > > The page fault can cause the current process to be de-scheduled, with > MSR[TS] active and without tm_recheckpoint() being called. More > importantly, without TEXASR[FS] bit set also. > > Since TEXASR might not have the FS bit set, and when the process is > scheduled back, it will try to reclaim, which will be aborted because of > the CPU is not in the suspended state, and, then, recheckpoint. This > recheckpoint will restore thread->texasr into TEXASR SPR, which might be > zero, hitting a BUG_ON(). > > kernel BUG at /build/linux-sf3Co9/linux-4.9.30/arch/powerpc/kernel/tm.S:434! > cpu 0xb: Vector: 700 (Program Check) at [c00000041f1576d0] > pc: c000000000054550: restore_gprs+0xb0/0x180 > lr: 0000000000000000 > sp: c00000041f157950 > msr: 8000000100021033 > current = 0xc00000041f143000 > paca = 0xc00000000fb86300 softe: 0 irq_happened: 0x01 > pid = 1021, comm = kworker/11:1 > kernel BUG at /build/linux-sf3Co9/linux-4.9.30/arch/powerpc/kernel/tm.S:434! > Linux version 4.9.0-3-powerpc64le (debian-kernel@lists.debian.org) (gcc version 6.3.0 20170516 (Debian 6.3.0-18) ) #1 SMP Debian 4.9.30-2+deb9u2 (2017-06-26) > enter ? for help > [c00000041f157b30] c00000000001bc3c tm_recheckpoint.part.11+0x6c/0xa0 > [c00000041f157b70] c00000000001d184 __switch_to+0x1e4/0x4c0 > [c00000041f157bd0] c00000000082eeb8 __schedule+0x2f8/0x990 > [c00000041f157cb0] c00000000082f598 schedule+0x48/0xc0 > [c00000041f157ce0] c0000000000f0d28 worker_thread+0x148/0x610 > [c00000041f157d80] c0000000000f96b0 kthread+0x120/0x140 > [c00000041f157e30] c00000000000c0e0 ret_from_kernel_thread+0x5c/0x7c > > This patch simply delays the MSR[TS] set, so, if there is any page fault in > the __get_user() section, it does not have regs->msr[TS] set, since the TM > structures are still invalid, thus avoiding doing TM operations for > in-kernel exceptions and possible process reschedule. > > With this patch, the MSR[TS] will only be set just before recheckpointing > and setting TEXASR[FS] = 1, thus avoiding an interrupt with TM registers in > invalid state. > > Other than that, if CONFIG_PREEMPT is set, there might be a preemption just > after setting MSR[TS] and before tm_recheckpoint(), thus, this block must > be atomic from a preemption perspective, thus, calling > preempt_disable/enable() on this code. > > It is not possible to move tm_recheckpoint to happen earlier, because it is > required to get the checkpointed registers from userspace, with > __get_user(), thus, the only way to avoid this undesired behavior is > delaying the MSR[TS] set. > > The 32-bits signal handler seems to be safe this current issue, but, it > might be exposed to the preemption issue, thus, disabling preemption in > this chunk of code. > > Changes from v2: > * Run the critical section with preempt_disable. > > Fixes: 87b4e5393af7 ("powerpc/tm: Fix return of active 64bit signals") > Cc: stable@vger.kernel.org (v3.9+) > Signed-off-by: Breno Leitao <leitao@debian.org> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/e1c3743e1a20647c53b719dbf28b48 cheers ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-08 10:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-21 19:21 [PATCH v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Breno Leitao 2018-11-28 13:23 ` [PATCH] selftests/powerpc: New TM signal self test Breno Leitao 2018-11-29 2:11 ` Michael Neuling 2018-12-04 17:51 ` Breno Leitao 2018-12-20 12:51 ` Michael Ellerman 2019-01-03 13:05 ` Breno Leitao 2019-01-08 10:16 ` Michael Ellerman 2018-12-23 13:27 ` [v2] powerpc/tm: Set MSR[TS] just prior to recheckpoint Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).