29 Jul
2016
29 Jul
'16
9:17 a.m.
This short series introduces fixes of 2 observed isues (#1 & #2) in clone()
syscall in ARC port and one enhancement (#3).
It all started from failing tst-clone1 in uClibc testsuite and during
its debugging we observed another quite subtle issue which was then not only
solved but better (which means shorter and simpler) implementation was made.
Alexey Brodkin (3):
arc: clone: Recover PID correctly
arc: clone: Fix CLONE_THREAD detection
arc: clone: Simplify CLONE_THREAD detection
libc/sysdeps/linux/arc/clone.S | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
--
2.7.4
29 Jul
29 Jul
9:17 a.m.
New subject: [PATCH 1/3] arc: clone: Recover PID correctly
Caught by tst-getpid1 test from uClibc's test-suite.
It looks like original implementation was not correct.
The code in question is supposed to recover PID of the new
thread. And by no means that could happen with clone() syscall
while getpid() does exactly this.
Signed-off-by: Alexey Brodkin <abrodkin(a)synopsys.com>
Acked-by: Vineet Gupta <vgupta(a)synopsys.com>
Reported-by: Eugeniy Paltsev <paltsev(a)synopsys.com>
---
libc/sysdeps/linux/arc/clone.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/sysdeps/linux/arc/clone.S b/libc/sysdeps/linux/arc/clone.S
index 3c1388e..dbb3fa7 100644
--- a/libc/sysdeps/linux/arc/clone.S
+++ b/libc/sysdeps/linux/arc/clone.S
@@ -72,7 +72,7 @@ ENTRY(clone)
and_s r2, r2, r12
brne r2, r12, .Lgo_thread
- mov r8, __NR_clone
+ mov r8, __NR_getpid
ARC_TRAP_INSN ; r0 has PID
THREAD_SELF r1 ; Get to struct pthread (just before TCB)
st r0, [r1, PTHREAD_PID]
--
2.7.4
9:17 a.m.
New subject: [PATCH 2/3] arc: clone: Fix CLONE_THREAD detection
For thread group case (CLONE_THREAD), the cached PID of new process/thread
need not be reset. The old logic to decide that was flawed as it would be
true only for exact combination of CLONE_THREAD + _VM, but would fail for
CLONE_THREAD + _VM + _xyz.
More detailed tear-down of current and new code below.
Current implementation is:
--------------------->8--------------------
; r12 contains clone flags
mov_s r2, CLONE_THREAD_N_VM; r2 contains bit mask
and_s r2, r2, r12 ; r2 contains bit mask AND clone flags
; but r12 still contains the same flags
brne r2, r12, .Lgo_thread ; here we compare modified mask with
; flags as they were and skip pthread TID/PID
; setup if r2 != r12 which happens all
; the time except clone flags were
; exactly CLONE_THREAD | CLONE_VM
--------------------->8--------------------
New implementation is:
--------------------->8--------------------
; r12 contains clone flags
mov_s r2, CLONE_THREAD_N_VM; r2 contains bit mask
and_s r12, r12, r2 ; r12 contains clone flags AND bit mask
; i.e. we did mask all flags except
; CLONE_THREAD and CLONE_VM
breq r2, r12, .Lgo_thread ; here we compare masked flags with
; target mask and if they match we skip
; pthread TID/PID setup
--------------------->8--------------------
Signed-off-by: Alexey Brodkin <abrodkin(a)synopsys.com>
Acked-by: Vineet Gupta <vgupta(a)synopsys.com>
---
libc/sysdeps/linux/arc/clone.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/sysdeps/linux/arc/clone.S b/libc/sysdeps/linux/arc/clone.S
index dbb3fa7..fc8dfcf 100644
--- a/libc/sysdeps/linux/arc/clone.S
+++ b/libc/sysdeps/linux/arc/clone.S
@@ -69,8 +69,8 @@ ENTRY(clone)
.Lnext_clone_quirk:
#ifdef RESET_PID
mov_s r2, CLONE_THREAD_N_VM
- and_s r2, r2, r12
- brne r2, r12, .Lgo_thread
+ and_s r12, r12, r2
+ breq r2, r12, .Lgo_thread
mov r8, __NR_getpid
ARC_TRAP_INSN ; r0 has PID
--
2.7.4
9:17 a.m.
New subject: [PATCH 3/3] arc: clone: Simplify CLONE_THREAD detection
This change was inspired by similar change in glibc:
https://sourceware.org/git/?p=glibc.git;a=commit;h=0cb313f7cb0e418b3d56f3a2…
Current Linux kernel requires CLONE_VM to be set with CLONE_THREAD
otherwise returning -EINVAL, see man clone2. This means we don't need
to check for both CLONE_THREAD and CLONE_VM instead we may simplify
code a lot and just check 1 bit (CLONE_THREAD).
Signed-off-by: Alexey Brodkin <abrodkin(a)synopsys.com>
Acked-by: Vineet Gupta <vgupta(a)synopsys.com>
---
libc/sysdeps/linux/arc/clone.S | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/libc/sysdeps/linux/arc/clone.S b/libc/sysdeps/linux/arc/clone.S
index fc8dfcf..3942b88 100644
--- a/libc/sysdeps/linux/arc/clone.S
+++ b/libc/sysdeps/linux/arc/clone.S
@@ -27,10 +27,7 @@
; void *tls,
; int __user *child_tidptr)
-#define CLONE_VM 0x00000100
-#define CLONE_THREAD 0x00010000
#define CLONE_SETTLS 0x00080000
-#define CLONE_THREAD_N_VM (CLONE_THREAD | CLONE_VM)
ENTRY(clone)
cmp r0, 0 ; @fn can't be NULL
@@ -68,9 +65,7 @@ ENTRY(clone)
.Lnext_clone_quirk:
#ifdef RESET_PID
- mov_s r2, CLONE_THREAD_N_VM
- and_s r12, r12, r2
- breq r2, r12, .Lgo_thread
+ bbit1 r12, 16, .Lgo_thread ; CLONE_THREAD = (1 << 16)
mov r8, __NR_getpid
ARC_TRAP_INSN ; r0 has PID
--
2.7.4
31 Jul
31 Jul
10:53 a.m.
New subject: [PATCH 0/3] Fixes and improvements for clone() syscall on ARC
Hi Alexey,
Alexey Brodkin wrote,
This short series introduces fixes of 2 observed isues
(#1 & #2) in clone()
syscall in ARC port and one enhancement (#3).
It all started from failing tst-clone1 in uClibc testsuite and during
its debugging we observed another quite subtle issue which was then not only
solved but better (which means shorter and simpler) implementation was made.
Alexey Brodkin (3):
arc: clone: Recover PID correctly
arc: clone: Fix CLONE_THREAD detection
arc: clone: Simplify CLONE_THREAD detection
Series applied and pushed,
best regards
Waldemar
3036
days inactive
3038
days old
4 comments
2 participants
participants (2)
-
Alexey Brodkin -
Waldemar Brodkorb