NAK: [PATCH Vivid SRU] powerpc/tm: backport b4b56f to Abort syscalls in active transactions
Kamal Mostafa
kamal at canonical.com
Tue May 10 15:52:07 UTC 2016
NAK for the following reasons:
- The patch author is wrong: The listed author needs to be the author
of the original patch, not the backporter.
Author: Sam bobroff <sam.bobroff at au1.ibm.com>
- The original patch description and Signed-off-by's from mainline have
been omitted and replaced with the backporter's notes and S-o-b.
Please re-submit this one with the complete original description and
author intact.
b4b56f9 powerpc/tm: Abort syscalls in active transactions
Thanks,
-Kamal
On Sun, May 08, 2016 at 05:17:24PM -0600, tim.gardner at canonical.com wrote:
> From: Simon Guo <simonguo at linux.vnet.ibm.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1572624
>
> This patch backport b4b56f9ecab40f3b to Ubuntu 14.04.04.
>
> quoted from original commit:
> It is about changing the syscall handler to doom (tabort) active
> transactions when a syscall is made and return very early without
> performing the syscall and keeping side effects to a minimum (no CPU
> accounting or system call tracing is performed). Also included is a
> new HWCAP2 bit, PPC_FEATURE2_HTM_NOSC, to indicate this
> behaviour to userspace.
>
> Without this backport patch, app running TM testing on Ubuntu 14.04
> might hang. Glibc with TLE enabled will be able to bypass this issue.
> However it is not enabled on Ubunt 14.04.
>
> The issue to be addressed:
> User is creating stack structure using C++ transactional memory
> extension:
>
> int Pop(int)
> {
> int ret = 0;
> __transaction_atomic
> {
> if(!stack_.empty())
> {
> ret = stack_.top();
> stack_.pop();
> } else
> ret = -1;
> }
> return ret;
> }
>
> While evaluating if(!stack_.empty()), this code calls a libitm function
> (GCC code), which calls malloc (glibc code) which ends up calling futex
> (a syscall).
> A syscall inside a transaction is forbidden by the kernel, but there is
> nothing the user can do to avoid this syscall.
>
> This will hang the user application inside the malloc(), which would be
> waiting for the futex to return.
>
> Ubuntu 14.04 provides glibc 2.19, which is too old to know about HTM.
> And this is probably happening with other libraries as well.
>
> Signed-off-by: Simon Guo <simonguo at linux.vnet.ibm.com>
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
> Documentation/powerpc/transactional_memory.txt | 32 +++++++++++------------
> arch/powerpc/include/asm/cputable.h | 10 +++++---
> arch/powerpc/include/uapi/asm/cputable.h | 1 +
> arch/powerpc/include/uapi/asm/tm.h | 2 +-
> arch/powerpc/kernel/cputable.c | 4 ++-
> arch/powerpc/kernel/entry_64.S | 35 ++++++++++++++++++++++++++
> 6 files changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/powerpc/transactional_memory.txt b/Documentation/powerpc/transactional_memory.txt
> index 9791e98..98b39af 100644
> --- a/Documentation/powerpc/transactional_memory.txt
> +++ b/Documentation/powerpc/transactional_memory.txt
> @@ -74,22 +74,23 @@ Causes of transaction aborts
> Syscalls
> ========
>
> -Performing syscalls from within transaction is not recommended, and can lead
> -to unpredictable results.
> +Syscalls made from within an active transaction will not be performed and the
> +transaction will be doomed by the kernel with the failure code TM_CAUSE_SYSCALL
> +| TM_CAUSE_PERSISTENT.
>
> -Syscalls do not by design abort transactions, but beware: The kernel code will
> -not be running in transactional state. The effect of syscalls will always
> -remain visible, but depending on the call they may abort your transaction as a
> -side-effect, read soon-to-be-aborted transactional data that should not remain
> -invisible, etc. If you constantly retry a transaction that constantly aborts
> -itself by calling a syscall, you'll have a livelock & make no progress.
> +Syscalls made from within a suspended transaction are performed as normal and
> +the transaction is not explicitly doomed by the kernel. However, what the
> +kernel does to perform the syscall may result in the transaction being doomed
> +by the hardware. The syscall is performed in suspended mode so any side
> +effects will be persistent, independent of transaction success or failure. No
> +guarantees are provided by the kernel about which syscalls will affect
> +transaction success.
>
> -Simple syscalls (e.g. sigprocmask()) "could" be OK. Even things like write()
> -from, say, printf() should be OK as long as the kernel does not access any
> -memory that was accessed transactionally.
> -
> -Consider any syscalls that happen to work as debug-only -- not recommended for
> -production use. Best to queue them up till after the transaction is over.
> +Care must be taken when relying on syscalls to abort during active transactions
> +if the calls are made via a library. Libraries may cache values (which may
> +give the appearance of success) or perform operations that cause transaction
> +failure before entering the kernel (which may produce different failure codes).
> +Examples are glibc's getpid() and lazy symbol resolution.
>
>
> Signals
> @@ -176,8 +177,7 @@ kernel aborted a transaction:
> TM_CAUSE_RESCHED Thread was rescheduled.
> TM_CAUSE_TLBI Software TLB invalide.
> TM_CAUSE_FAC_UNAV FP/VEC/VSX unavailable trap.
> - TM_CAUSE_SYSCALL Currently unused; future syscalls that must abort
> - transactions for consistency will use this.
> + TM_CAUSE_SYSCALL Syscall from active transaction.
> TM_CAUSE_SIGNAL Signal delivered.
> TM_CAUSE_MISC Currently unused.
> TM_CAUSE_ALIGNMENT Alignment fault.
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 22d5a7d..4ec19da 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -236,11 +236,13 @@ extern const char *powerpc_base_platform;
>
> /* We only set the TM feature if the kernel was compiled with TM supprt */
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> -#define CPU_FTR_TM_COMP CPU_FTR_TM
> -#define PPC_FEATURE2_HTM_COMP PPC_FEATURE2_HTM
> +#define CPU_FTR_TM_COMP CPU_FTR_TM
> +#define PPC_FEATURE2_HTM_COMP PPC_FEATURE2_HTM
> +#define PPC_FEATURE2_HTM_NOSC_COMP PPC_FEATURE2_HTM_NOSC
> #else
> -#define CPU_FTR_TM_COMP 0
> -#define PPC_FEATURE2_HTM_COMP 0
> +#define CPU_FTR_TM_COMP 0
> +#define PPC_FEATURE2_HTM_COMP 0
> +#define PPC_FEATURE2_HTM_NOSC_COMP 0
> #endif
>
> /* We need to mark all pages as being coherent if we're SMP or we have a
> diff --git a/arch/powerpc/include/uapi/asm/cputable.h b/arch/powerpc/include/uapi/asm/cputable.h
> index 67de80a..2734c00 100644
> --- a/arch/powerpc/include/uapi/asm/cputable.h
> +++ b/arch/powerpc/include/uapi/asm/cputable.h
> @@ -43,5 +43,6 @@
> #define PPC_FEATURE2_ISEL 0x08000000
> #define PPC_FEATURE2_TAR 0x04000000
> #define PPC_FEATURE2_VEC_CRYPTO 0x02000000
> +#define PPC_FEATURE2_HTM_NOSC 0x01000000
>
> #endif /* _UAPI__ASM_POWERPC_CPUTABLE_H */
> diff --git a/arch/powerpc/include/uapi/asm/tm.h b/arch/powerpc/include/uapi/asm/tm.h
> index 5d836b7..5047659 100644
> --- a/arch/powerpc/include/uapi/asm/tm.h
> +++ b/arch/powerpc/include/uapi/asm/tm.h
> @@ -11,7 +11,7 @@
> #define TM_CAUSE_RESCHED 0xde
> #define TM_CAUSE_TLBI 0xdc
> #define TM_CAUSE_FAC_UNAV 0xda
> -#define TM_CAUSE_SYSCALL 0xd8 /* future use */
> +#define TM_CAUSE_SYSCALL 0xd8
> #define TM_CAUSE_MISC 0xd6 /* future use */
> #define TM_CAUSE_SIGNAL 0xd4
> #define TM_CAUSE_ALIGNMENT 0xd2
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index c18a2ea..bf3eb39 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -108,7 +108,9 @@ extern void __restore_cpu_e6500(void);
> PPC_FEATURE_TRUE_LE | \
> PPC_FEATURE_PSERIES_PERFMON_COMPAT)
> #define COMMON_USER2_POWER8 (PPC_FEATURE2_ARCH_2_07 | \
> - PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_DSCR | \
> + PPC_FEATURE2_HTM_COMP | \
> + PPC_FEATURE2_HTM_NOSC_COMP | \
> + PPC_FEATURE2_DSCR | \
> PPC_FEATURE2_ISEL | PPC_FEATURE2_TAR | \
> PPC_FEATURE2_VEC_CRYPTO)
> #define COMMON_USER_PA6T (COMMON_USER_PPC64 | PPC_FEATURE_PA6T |\
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 1d8b58a..0dce87f 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -34,6 +34,7 @@
> #include <asm/ftrace.h>
> #include <asm/hw_irq.h>
> #include <asm/context_tracking.h>
> +#include <asm/tm.h>
>
> /*
> * System calls.
> @@ -51,6 +52,12 @@ exception_marker:
>
> .globl system_call_common
> system_call_common:
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +BEGIN_FTR_SECTION
> + extrdi. r10, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */
> + bne tabort_syscall
> +END_FTR_SECTION_IFSET(CPU_FTR_TM)
> +#endif
> andi. r10,r12,MSR_PR
> mr r10,r1
> addi r1,r1,-INT_FRAME_SIZE
> @@ -311,6 +318,34 @@ syscall_exit_work:
> bl do_syscall_trace_leave
> b ret_from_except
>
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +tabort_syscall:
> + /* Firstly we need to enable TM in the kernel */
> + mfmsr r10
> + li r13, 1
> + rldimi r10, r13, MSR_TM_LG, 63-MSR_TM_LG
> + mtmsrd r10, 0
> +
> + /* tabort, this dooms the transaction, nothing else */
> + li r13, (TM_CAUSE_SYSCALL|TM_CAUSE_PERSISTENT)
> + TABORT(R13)
> +
> + /*
> + * Return directly to userspace. We have corrupted user register state,
> + * but userspace will never see that register state. Execution will
> + * resume after the tbegin of the aborted transaction with the
> + * checkpointed register state.
> + */
> + li r13, MSR_RI
> + andc r10, r10, r13
> + mtmsrd r10, 1
> + mtspr SPRN_SRR0, r11
> + mtspr SPRN_SRR1, r12
> +
> + rfid
> + b . /* prevent speculative execution */
> +#endif
> +
> /* Save non-volatile GPRs, if not already saved. */
> _GLOBAL(save_nvgprs)
> ld r11,_TRAP(r1)
> --
> 1.9.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list