diff mbox series

[v5,22/22] loongarch: Fix Race conditions in pthread cancellation [BZ#12683]

Message ID 20230410204614.4129551-14-adhemerval.zanella@linaro.org
State Superseded
Headers
Series Fix Race conditions in pthread cancellation [BZ#12683] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Adhemerval Zanella April 10, 2023, 8:46 p.m. UTC 
  By adding the required syscall_cancel.S.

Checked against a build and make check run-built-tests=no for
loongarch64-linux-gnu-lp64d.
---
 .../sysv/linux/loongarch/syscall_cancel.S     | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/loongarch/syscall_cancel.S

Comments

Xi Ruoyao April 11, 2023, 6:19 a.m. UTC | #1 
On Mon, 2023-04-10 at 17:46 -0300, Adhemerval Zanella via Libc-alpha
wrote:

/* snip */

> +1:
> +       addi.d  $r3,$r3,-16
> +       cfi_def_cfa_offset (16)
> +       st.d    $r1,$r3,8
> +       cfi_offset (1, -8)
> +       bl      __syscall_do_cancel

Can we simply use "b __syscall_do_cancel" for 1: like AArch64?

> +
> +END (__syscall_cancel_arch)
caiyinyu April 11, 2023, noon UTC | #2 
在 2023/4/11 下午2:19, Xi Ruoyao 写道:
> On Mon, 2023-04-10 at 17:46 -0300, Adhemerval Zanella via Libc-alpha
> wrote:
>
> /* snip */
>
>> +1:
>> +       addi.d  $r3,$r3,-16
>> +       cfi_def_cfa_offset (16)
>> +       st.d    $r1,$r3,8
>> +       cfi_offset (1, -8)
>> +       bl      __syscall_do_cancel
> Can we simply use "b __syscall_do_cancel" for 1: like AArch64?

In LoongArch, the "b" instruction is equivalent to the "b" instruction 
in AArch64, and similarly,

the "bl" instruction in LoongArch is equivalent to the "bl" instruction 
in AArch64.

>> +
>> +END (__syscall_cancel_arch)
caiyinyu April 11, 2023, 12:21 p.m. UTC | #3 
在 2023/4/11 下午8:00, caiyinyu 写道:
>
> 在 2023/4/11 下午2:19, Xi Ruoyao 写道:
>> On Mon, 2023-04-10 at 17:46 -0300, Adhemerval Zanella via Libc-alpha
>> wrote:
>>
>> /* snip */
>>
>>> +1:
>>> +       addi.d  $r3,$r3,-16
>>> +       cfi_def_cfa_offset (16)
>>> +       st.d    $r1,$r3,8
>>> +       cfi_offset (1, -8)
>>> +       bl      __syscall_do_cancel
>> Can we simply use "b __syscall_do_cancel" for 1: like AArch64?
>
> In LoongArch, the "b" instruction is equivalent to the "b" instruction 
> in AArch64, and similarly,
>
> the "bl" instruction in LoongArch is equivalent to the "bl" 
> instruction in AArch64.

This is our instruction manual.

https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#arithmetic-operation-instructions


>
>>> +
>>> +END (__syscall_cancel_arch)
Xi Ruoyao April 11, 2023, 12:55 p.m. UTC | #4 
On Tue, 2023-04-11 at 20:00 +0800, caiyinyu wrote:
> 
> 在 2023/4/11 下午2:19, Xi Ruoyao 写道:
> > On Mon, 2023-04-10 at 17:46 -0300, Adhemerval Zanella via Libc-alpha
> > wrote:
> > 
> > /* snip */
> > 
> > > +1:
> > > +       addi.d  $r3,$r3,-16
> > > +       cfi_def_cfa_offset (16)
> > > +       st.d    $r1,$r3,8
> > > +       cfi_offset (1, -8)
> > > +       bl      __syscall_do_cancel
> > Can we simply use "b __syscall_do_cancel" for 1: like AArch64?
> 
> In LoongArch, the "b" instruction is equivalent to the "b" instruction
> in AArch64, and similarly,
> 
> the "bl" instruction in LoongArch is equivalent to the "bl"
> instruction 
> in AArch64.
> 
> > > +
> > > +END (__syscall_cancel_arch)

I've rewritten the syscall_cancel.S file as follow to exploit the tail
call and improve readability.  It passed nptl/tst-cancel31:

#include <sysdep.h>
#include <descr-const.h>

ENTRY (__syscall_cancel_arch)

	.global __syscall_cancel_arch_start
__syscall_cancel_arch_start:

	/* if (*cancelhandling & CANCELED_BITMASK)
	     __syscall_do_cancel()  */
	ld.w	t0, a0, 0
	andi	t0, t0, TCB_CANCELED_BITMASK
	bnez	t0, 1f

	/* Issue a 6 argument syscall.  */
	move	t1, a1
	move	a0, a2
	move	a1, a3
	move	a2, a4
	move	a3, a5
	move	a4, a6
	move	a5, a7
	move	a7, t1
	syscall 0

	.global __syscall_cancel_arch_end
__syscall_cancel_arch_end:
	jr	ra
1:
	b	__syscall_do_cancel

END (__syscall_cancel_arch)

>
Adhemerval Zanella April 11, 2023, 1:54 p.m. UTC | #5 
On 11/04/23 09:55, Xi Ruoyao wrote:
> On Tue, 2023-04-11 at 20:00 +0800, caiyinyu wrote:
>>
>> 在 2023/4/11 下午2:19, Xi Ruoyao 写道:
>>> On Mon, 2023-04-10 at 17:46 -0300, Adhemerval Zanella via Libc-alpha
>>> wrote:
>>>
>>> /* snip */
>>>
>>>> +1:
>>>> +       addi.d  $r3,$r3,-16
>>>> +       cfi_def_cfa_offset (16)
>>>> +       st.d    $r1,$r3,8
>>>> +       cfi_offset (1, -8)
>>>> +       bl      __syscall_do_cancel
>>> Can we simply use "b __syscall_do_cancel" for 1: like AArch64?
>>
>> In LoongArch, the "b" instruction is equivalent to the "b" instruction
>> in AArch64, and similarly,
>>
>> the "bl" instruction in LoongArch is equivalent to the "bl"
>> instruction 
>> in AArch64.
>>
>>>> +
>>>> +END (__syscall_cancel_arch)
> 
> I've rewritten the syscall_cancel.S file as follow to exploit the tail
> call and improve readability.  It passed nptl/tst-cancel31:
> 
> #include <sysdep.h>
> #include <descr-const.h>
> 
> ENTRY (__syscall_cancel_arch)
> 
> 	.global __syscall_cancel_arch_start
> __syscall_cancel_arch_start:
> 
> 	/* if (*cancelhandling & CANCELED_BITMASK)
> 	     __syscall_do_cancel()  */
> 	ld.w	t0, a0, 0
> 	andi	t0, t0, TCB_CANCELED_BITMASK
> 	bnez	t0, 1f
> 
> 	/* Issue a 6 argument syscall.  */
> 	move	t1, a1
> 	move	a0, a2
> 	move	a1, a3
> 	move	a2, a4
> 	move	a3, a5
> 	move	a4, a6
> 	move	a5, a7
> 	move	a7, t1
> 	syscall 0
> 
> 	.global __syscall_cancel_arch_end
> __syscall_cancel_arch_end:
> 	jr	ra
> 1:
> 	b	__syscall_do_cancel
> 
> END (__syscall_cancel_arch)
> 

Thanks, I have merged it locally.  I will update my sourceware branch
with this change.
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/loongarch/syscall_cancel.S b/sysdeps/unix/sysv/linux/loongarch/syscall_cancel.S
new file mode 100644
index 0000000000..574252fec1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/loongarch/syscall_cancel.S
@@ -0,0 +1,54 @@ 
+/* Cancellable syscall wrapper.  Linux/loongarch version.
+   Copyright (C) 2023 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/>.  */
+
+#include <sysdep.h>
+#include <descr-const.h>
+
+ENTRY (__syscall_cancel_arch)
+
+	.global __syscall_cancel_arch_start
+__syscall_cancel_arch_start:
+
+	/* if (*cancelhandling & CANCELED_BITMASK)
+	     __syscall_do_cancel()  */
+	ldptr.w	$r12,$r4,0
+	andi	$r12,$r12,TCB_CANCELED_BITMASK
+	bnez	$r12,1f
+
+	/* Issue a 6 argument syscall.  */
+	or	$r13,$r5,$r0
+	or	$r4,$r6,$r0
+	or	$r5,$r7,$r0
+	or	$r6,$r8,$r0
+	or	$r7,$r9,$r0
+	or	$r8,$r10,$r0
+	or	$r9,$r11,$r0
+	or	$r11,$r13,$r0
+	syscall 0
+
+	.global __syscall_cancel_arch_end
+__syscall_cancel_arch_end:
+	jr	$r1
+1:
+	addi.d	$r3,$r3,-16
+	cfi_def_cfa_offset (16)
+	st.d	$r1,$r3,8
+	cfi_offset (1, -8)
+	bl	__syscall_do_cancel
+
+END (__syscall_cancel_arch)