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

Message ID 20180112160704.20107-1-tuliom@linux.vnet.ibm.com
State New
Headers show
Series
  • [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]
Related show

Commit Message

Tulio Magno Quites Machado Filho Jan. 12, 2018, 4:07 p.m.
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/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/unix/sysv/linux/powerpc/not-errno.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/not-errno.h

Comments

Florian Weimer Jan. 12, 2018, 4:31 p.m. | #1
On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
> +/* __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"

This #define isn't properly scoped, so I really don't like this approach 
for a header file.

Thanks,
Florian
Tulio Magno Quites Machado Filho Jan. 12, 2018, 4:40 p.m. | #2
Florian Weimer <fweimer@redhat.com> writes:

> On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
>> +/* __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"
>
> This #define isn't properly scoped, so I really don't like this approach 
> for a header file.

Do you think that checking for TUNABLES_INTERNAL would be enough?
Florian Weimer Jan. 12, 2018, 4:44 p.m. | #3
On 01/12/2018 05:40 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> 
>> On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
>>> +/* __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"
>>
>> This #define isn't properly scoped, so I really don't like this approach
>> for a header file.
> 
> Do you think that checking for TUNABLES_INTERNAL would be enough?

I'm not sure what you mean.

If you move the current definition of ABORT_TRANSACTION to 
ABORT_TRANSACTION_IMPL, you could perhaps do

#undef ABORT_TRANSACTION
#define ABORT_TRANSACTION

#include "sysdeps/unix/sysv/linux/not-errno.h"

#undef ABORT_TRANSACTION
#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL

(with comments, obviously).

Thanks,
Florian

Patch

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..642cb8a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
@@ -0,0 +1,25 @@ 
+/* 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"