23 May
2016
23 May
'16
10:38 a.m.
Hi,
I'm propose to take a glibc's commit 6d96f5e4c0 to solve the
unnecessary busy loop in __lll_timedlock_wait on ARM, described in
details at:
https://sourceware.org/bugzilla/show_bug.cgi?id=15119
https://patchwork.ozlabs.org/patch/332656/
what is your opinion?
Regards,
Leonid
23 May
23 May
11:18 a.m.
Hi Leonid,
Leonid Lisovskiy wrote,
Hi,
I'm propose to take a glibc's commit 6d96f5e4c0 to solve the
unnecessary busy loop in __lll_timedlock_wait on ARM, described in
details at:
https://sourceware.org/bugzilla/show_bug.cgi?id=15119
https://patchwork.ozlabs.org/patch/332656/
what is your opinion?
So you remove the specific lowlevellock.c from ARM and take that the
generic works fine?
best regards
Waldemar
24 May
24 May
7:18 a.m.
On Mon, May 23, 2016 at 2:18 PM, Waldemar Brodkorb <wbx(a)uclibc-ng.org> wrote:
So you remove the specific lowlevellock.c from ARM and
take that the
generic works fine?
With appropriate Makefile.arch changes, of course.
I haven't real hardware, but basic tests runs under QEMU fine.
Moreover, it tested by glibc users since generic lowlevellock.c
exactly the same as ours.
Leonid
7:32 a.m.
Hi Leonid,
Leonid Lisovskiy wrote,
On Mon, May 23, 2016 at 2:18 PM, Waldemar Brodkorb
<wbx(a)uclibc-ng.org> wrote:
I am fine with this change, removing unneccessary architecture
specific files is a good thing.
best regards
Waldemar
So you remove the specific lowlevellock.c from
ARM and take that the
generic works fine?
With appropriate Makefile.arch changes, of course.
I haven't real hardware, but basic tests runs under QEMU fine.
Moreover, it tested by glibc users since generic lowlevellock.c
exactly the same as ours.
26 May
26 May
7:53 a.m.
On Tue, May 24, 2016 at 10:32 AM, Waldemar Brodkorb <wbx(a)uclibc-ng.org> wrote:
I am fine with this change, removing unneccessary
architecture
specific files is a good thing.
Should I prepare the patch against the trunk? If yes, should I keep
the original author of glibc commit ("Will Newton")?
Leonid
8:04 a.m.
Hi,
Leonid Lisovskiy wrote,
On Tue, May 24, 2016 at 10:32 AM, Waldemar Brodkorb
<wbx(a)uclibc-ng.org> wrote:
Trunk is good.
As you like, you can mention the commit from Glibc, should be
enough.
best regards
Waldemar
I am fine with this change, removing unneccessary
architecture
specific files is a good thing.
Should I prepare the patch against the trunk? If yes, should I keep
the original author of glibc commit ("Will Newton")?
30 May
30 May
5:19 p.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
lowlevellock.c for arm differs from the generic lowlevellock.c only in
insignificant ways, so can be removed. Follow glibc commit 6d96f5e4c0
Solves __lll_timedlock_wait busy-wait issues described at
http://sourceware.org/bugzilla/show_bug.cgi?id=15119
Signed-off-by: Leonid Lisovskiy <lly.dev(a)gmail.com>
---
.../nptl/sysdeps/unix/sysv/linux/arm/Makefile.arch | 5 +-
.../unix/sysv/linux/arm/libc-lowlevellock.c | 20 ---
.../sysdeps/unix/sysv/linux/arm/lowlevellock.c | 136 ---------------------
3 files changed, 2 insertions(+), 159 deletions(-)
delete mode 100644 libpthread/nptl/sysdeps/unix/sysv/linux/arm/libc-lowlevellock.c
delete mode 100644 libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/Makefile.arch
b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/Makefile.arch
index 5522ce3..80a0306 100644
--- a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/Makefile.arch
+++ b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/Makefile.arch
@@ -7,10 +7,9 @@
libpthread_linux_arch_SSRC =
libpthread_linux_arch_CSRC = pthread_once.c \
- pt-__syscall_rt_sigaction.c pt-__syscall_error.c \
- lowlevellock.c
+ pt-__syscall_rt_sigaction.c pt-__syscall_error.c
-libc_linux_arch_CSRC = fork.c libc-lowlevellock.c
+libc_linux_arch_CSRC = fork.c
libc_linux_arch_SSRC = clone.S vfork.S
libc_linux_arch_SSRC-OMIT = waitpid.S
diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/libc-lowlevellock.c
b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/libc-lowlevellock.c
deleted file mode 100644
index 28672a6..0000000
--- a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/libc-lowlevellock.c
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Copyright (C) 2003 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Contributed by Paul Mackerras <paulus(a)au.ibm.com>om>, 2003.
-
- 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/>. */
-
-/* No difference to lowlevellock.c, except we lose a couple of functions. */
-#include "lowlevellock.c"
diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
b/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
deleted file mode 100644
index cd42135..0000000
--- a/libpthread/nptl/sysdeps/unix/sysv/linux/arm/lowlevellock.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/* low level locking for pthread library. Generic futex-using version.
- Copyright (C) 2003, 2005, 2007 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 <errno.h>
-#include <sysdep.h>
-#include <lowlevellock.h>
-#include <sys/time.h>
-#include <tls.h>
-
-void
-#ifndef IS_IN_libpthread
-weak_function
-#endif
-__lll_lock_wait_private (int *futex)
-{
- do
- {
- int oldval = atomic_compare_and_exchange_val_acq (futex, 2, 1);
- if (oldval != 0)
- lll_futex_wait (futex, 2, LLL_PRIVATE);
- }
- while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0);
-}
-
-
-/* These functions don't get included in libc.so */
-#ifdef IS_IN_libpthread
-void
-__lll_lock_wait (int *futex, int private)
-{
- do
- {
- int oldval = atomic_compare_and_exchange_val_acq (futex, 2, 1);
- if (oldval != 0)
- lll_futex_wait (futex, 2, private);
- }
- while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0);
-}
-
-
-int
-__lll_timedlock_wait (int *futex, const struct timespec *abstime, int private)
-{
- struct timespec rt;
-
- /* Reject invalid timeouts. */
- if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
- return EINVAL;
-
- /* Upgrade the lock. */
- if (atomic_exchange_acq (futex, 2) == 0)
- return 0;
-
- do
- {
- struct timeval tv;
-
- /* Get the current time. */
- (void) gettimeofday (&tv, NULL);
-
- /* Compute relative timeout. */
- rt.tv_sec = abstime->tv_sec - tv.tv_sec;
- rt.tv_nsec = abstime->tv_nsec - tv.tv_usec * 1000;
- if (rt.tv_nsec < 0)
- {
- rt.tv_nsec += 1000000000;
- --rt.tv_sec;
- }
-
- /* Already timed out? */
- if (rt.tv_sec < 0)
- return ETIMEDOUT;
-
- // XYZ: Lost the lock to check whether it was private.
- lll_futex_timed_wait (futex, 2, &rt, private);
- }
- while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0);
-
- return 0;
-}
-
-
-int
-__lll_timedwait_tid (int *tidp, const struct timespec *abstime)
-{
- int tid;
-
- if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
- return EINVAL;
-
- /* Repeat until thread terminated. */
- while ((tid = *tidp) != 0)
- {
- struct timeval tv;
- struct timespec rt;
-
- /* Get the current time. */
- (void) gettimeofday (&tv, NULL);
-
- /* Compute relative timeout. */
- rt.tv_sec = abstime->tv_sec - tv.tv_sec;
- rt.tv_nsec = abstime->tv_nsec - tv.tv_usec * 1000;
- if (rt.tv_nsec < 0)
- {
- rt.tv_nsec += 1000000000;
- --rt.tv_sec;
- }
-
- /* Already timed out? */
- if (rt.tv_sec < 0)
- return ETIMEDOUT;
-
- /* Wait until thread terminates. */
- // XYZ: Lost the lock to check whether it was private.
- if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT)
- return ETIMEDOUT;
- }
-
- return 0;
-}
-#endif
--
1.8.5.6
2 Jun
2 Jun
6:58 p.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
Hi Leonid,
Leonid Lisovskiy wrote,
lowlevellock.c for arm differs from the generic
lowlevellock.c only in
insignificant ways, so can be removed. Follow glibc commit 6d96f5e4c0
Solves __lll_timedlock_wait busy-wait issues described at
http://sourceware.org/bugzilla/show_bug.cgi?id=15119
Signed-off-by: Leonid Lisovskiy <lly.dev(a)gmail.com>
Thanks, applied and pushed,
Waldemar
3 Jun
3 Jun
8:27 a.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
On Thu, Jun 2, 2016 at 9:58 PM, Waldemar Brodkorb <wbx(a)uclibc-ng.org> wrote:
I suggest to try to repeat the same action for other platforms using
local lowlevellock.c copy. Starting from xtensa, since it don't use it
at all(no reference to lowlevellock.c in Makefile.in), then - arc,
metag, sparc.
But, unfortunately, I can't make the tests on these platforms myself.
Waldemar, could you help with testing?
Regards,
Leonid
lowlevellock.c
for arm differs from the generic lowlevellock.c only in
insignificant ways, so can be removed. Follow glibc commit 6d96f5e4c0
Thanks, applied and pushed,
Waldemar
3:28 p.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
Hi Leonid,
Leonid Lisovskiy wrote,
On Thu, Jun 2, 2016 at 9:58 PM, Waldemar Brodkorb
<wbx(a)uclibc-ng.org> wrote:
I suggest to try to repeat the same action for other platforms using
local lowlevellock.c copy. Starting from xtensa, since it don't use it
at all(no reference to lowlevellock.c in Makefile.in), then - arc,
metag, sparc.
But, unfortunately, I can't make the tests on these platforms myself.
Waldemar, could you help with testing?
If you provide one patch for each architecture I can test them and
if no regression happens I can commit them.
Do you think our testsuite covers the included functionality in
lowlevellock.c enough, so that no failure count changes, mean
the generic code is fine?
best regards
Waldemar
lowlevellock.c for arm differs from the generic lowlevellock.c only in
insignificant ways, so can be removed. Follow glibc commit 6d96f5e4c0
Thanks, applied and pushed,
Waldemar
7 Jun
7 Jun
8:39 a.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
On Fri, Jun 3, 2016 at 6:28 PM, Waldemar Brodkorb <wbx(a)uclibc-ng.org> wrote:
If you provide one patch for each architecture I can
test them and
if no regression happens I can commit them.
Excellent! Patches attached.
The single platform that keep local lowlevellock.c is sparc, like
glibc (sysdeps/sparc/sparc32/lowlevellock.c).
Do you think our testsuite covers the included
functionality in
lowlevellock.c enough, so that no failure count changes, mean
the generic code is fine?
Testsuite cover standard, predictable, situations. But I'm unsure
about races & other unusual problems tests.
Yes, this adds some potential risk, but I haven't any good idea how to
eliminate it at all.
regards,
Leonid
12 Jun
12 Jun
9:47 a.m.
New subject: [PATCH] ARM: nptl: Use generic lowlevellock.c, patch from glibc
Hi Leonid,
Leonid Lisovskiy wrote,
On Fri, Jun 3, 2016 at 6:28 PM, Waldemar Brodkorb
<wbx(a)uclibc-ng.org> wrote:
Thanks, I can live with this risk.
Tested and pushed,
Waldemar
If you provide one patch for each architecture I
can test them and
if no regression happens I can commit them.
Excellent! Patches attached.
The single platform that keep local lowlevellock.c is sparc, like
glibc (sysdeps/sparc/sparc32/lowlevellock.c).
Do you think our testsuite covers the included
functionality in
lowlevellock.c enough, so that no failure count changes, mean
the generic code is fine?
Testsuite cover standard, predictable, situations. But I'm unsure
about races & other unusual problems tests.
Yes, this adds some potential risk, but I haven't any good idea how to
eliminate it at all.
2875
days inactive
2895
days old
11 comments
2 participants
participants (2)
-
Leonid Lisovskiy -
Waldemar Brodkorb