Patchwork [PATCHv3] powerpc: Fix syscalls during early process initialization [BZ #22685]

login
register
mail settings
Submitter Tulio Magno Quites Machado Filho
Date Jan. 12, 2018, 8:52 p.m.
Message ID <20180112205228.21263-1-tuliom@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/25370/
State Committed
Delegated to: Aurelien Jarno
Headers show

Comments

Tulio Magno Quites Machado Filho - Jan. 12, 2018, 8:52 p.m.
Changes since v2:
 - Limited the scope of the changes to ABORT_TRANSACTION.

Changes since v1:
 - Re-implemented the patch to re-define a powerpc-specific
   __access_noerrno.

--- 8< ---

The tunables framework needs to execute syscall early in process
initialization, before the TCB is available for consumption.  This
behavior conflicts with powerpc{|64|64le}'s lock elision code, that
checks the TCB before trying to abort transactions immediately before
executing a syscall.

This patch adds a powerpc-specific implementation of __access_noerrno
that does not abort transactions before the executing syscall.

Tested on powerpc{|64|64le}.

2018-01-12  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>

	[BZ #22685]
	* sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION_IMPL): Renamed
	from ABORT_TRANSACTION.
	(ABORT_TRANSACTION): Redirect to ABORT_TRANSACTION_IMPL.
	* sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION,
	ABORT_TRANSACTION_IMPL): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/not-errno.h: New file.  Reuse
	Linux code, but remove the code that aborts transactions.

Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
---
 sysdeps/powerpc/powerpc32/sysdep.h          |  5 +++--
 sysdeps/powerpc/powerpc64/sysdep.h          |  5 +++--
 sysdeps/unix/sysv/linux/powerpc/not-errno.h | 30 +++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/not-errno.h
Aurelien Jarno - Jan. 16, 2018, 3:51 p.m.
On 2018-01-12 18:52, Tulio Magno Quites Machado Filho wrote:
> Changes since v2:
>  - Limited the scope of the changes to ABORT_TRANSACTION.
> 
> Changes since v1:
>  - Re-implemented the patch to re-define a powerpc-specific
>    __access_noerrno.
> 
> --- 8< ---
> 
> The tunables framework needs to execute syscall early in process
> initialization, before the TCB is available for consumption.  This
> behavior conflicts with powerpc{|64|64le}'s lock elision code, that
> checks the TCB before trying to abort transactions immediately before
> executing a syscall.
> 
> This patch adds a powerpc-specific implementation of __access_noerrno
> that does not abort transactions before the executing syscall.
> 
> Tested on powerpc{|64|64le}.
> 
> 2018-01-12  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
> 
> 	[BZ #22685]
> 	* sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION_IMPL): Renamed
> 	from ABORT_TRANSACTION.
> 	(ABORT_TRANSACTION): Redirect to ABORT_TRANSACTION_IMPL.
> 	* sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION,
> 	ABORT_TRANSACTION_IMPL): Likewise.
> 	* sysdeps/unix/sysv/linux/powerpc/not-errno.h: New file.  Reuse
> 	Linux code, but remove the code that aborts transactions.

Thanks for this new version. It is indeed much simpler than the initial
version. I confirm it fixes the issue and does not introduce regressions
in the testsuite.

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

Patch

diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 62c018f..8e32a2a 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -91,7 +91,7 @@  GOT_LABEL:			;					      \
   ASM_SIZE_DIRECTIVE(name)
 
 #if ! IS_IN(rtld)
-# define ABORT_TRANSACTION \
+# define ABORT_TRANSACTION_IMPL \
     cmpwi    2,0;		\
     beq      1f;		\
     lwz      0,TM_CAPABLE(2);	\
@@ -102,8 +102,9 @@  GOT_LABEL:			;					      \
     .align 4;			\
 1:
 #else
-# define ABORT_TRANSACTION
+# define ABORT_TRANSACTION_IMPL
 #endif
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
 
 #define DO_CALL(syscall)						      \
     ABORT_TRANSACTION							      \
diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index 26b0885..2df1d9b 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -264,7 +264,7 @@  LT_LABELSUFFIX(name,_name_end): ; \
   END_2(name)
 
 #if !IS_IN(rtld)
-# define ABORT_TRANSACTION \
+# define ABORT_TRANSACTION_IMPL \
     cmpdi    13,0;		\
     beq      1f;		\
     lwz      0,TM_CAPABLE(13);	\
@@ -275,8 +275,9 @@  LT_LABELSUFFIX(name,_name_end): ; \
     .p2align 4;			\
 1:
 #else
-# define ABORT_TRANSACTION
+# define ABORT_TRANSACTION_IMPL
 #endif
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
 
 #define DO_CALL(syscall) \
     ABORT_TRANSACTION \
diff --git a/sysdeps/unix/sysv/linux/powerpc/not-errno.h b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
new file mode 100644
index 0000000..27da21b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
@@ -0,0 +1,30 @@ 
+/* Syscall wrapper that do not set errno.  Linux powerpc version.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* __access_noerrno is used during process initialization in elf/dl-tunables.c
+   before the TCB is initialized, prohibiting the usage of
+   ABORT_TRANSACTION.  */
+#undef ABORT_TRANSACTION
+#define ABORT_TRANSACTION
+
+#include "sysdeps/unix/sysv/linux/not-errno.h"
+
+/* Recover ABORT_TRANSACTION's previous value, in order to not affect
+   other syscalls.  */
+#undef ABORT_TRANSACTION
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL