Message ID | CALE0ps19HNFvdrD0j0BPhx6tyBW+DuyKbYEMXJ513FqUeYPDEg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 04/28/2014 11:51 AM, Bernie Ogden wrote: > lowlevellock.c for hppa differs from the generic lowlevellock.c only in > insignificant ways, so can be removed. I don't have any hppa targets > to work with, so have not been able to test this patch. Thanks for this. I'll test this shortly. > The notable differences between the hppa and generic implementations are: > > 1) Some functions in hppa's lowlevellock.c set futex to 2 if it was 1. > The generic version always sets the futex to 2. As futex can only be > 0, 1 or 2 on entry into these functions, the behaviour is equivalent. > (If the futex manages to be 0 on entry then we've just lost an > unlikely fast path out.) OK. > 2) hppa places most of the functions in this file in both libc and > libpthread. The generic implementation places only > __lll_lock_wait_private in libc. As these are internal functions I > think that, if this does cause a problem, it would show up at build > time - but I'm new to glibc so prepared to be corrected (and have not > been able to build). I'll test this, but as they are internal functions it probably doesn't matter but makes the final object size smaller. > I would be grateful if the maintainer could test/comment. > > Regards, > > Bernie > > ports/ChangeLog.hppa > 2014-04-24 Bernard Ogden <bernie.ogden@linaro.org> > > [BZ #15119] > * ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c: Remove file. Slightly the wrong format. Should be: 2014-04-24 Bernard Ogden <bernie.ogden@linaro.org> [BZ #15119] * ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c: Remove file. However, I haven't moved hppa out of ports, I'll do that at the end of the day or if I get a break of time waiting on something. > diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c > b/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c > deleted file mode 100644 > index d61c5d3..0000000 > --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c > +++ /dev/null > @@ -1,127 +0,0 @@ > -/* low level locking for pthread library. Generic futex-using version. > - Copyright (C) 2003-2014 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Contributed by Paul Mackerras <paulus@au.ibm.com>, 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/>. */ > - > -#include <errno.h> > -#include <sysdep.h> > -#include <lowlevellock.h> > -#include <sys/time.h> > - > -void > -__lll_lock_wait (lll_lock_t *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_val_acq (futex, 2, 0) != 0); > -} > - > -void > -__lll_lock_wait_private (lll_lock_t *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_val_acq (futex, 2, 0) != 0); > -} > - > -int > -__lll_timedlock_wait (lll_lock_t *futex, const struct timespec > *abstime, int private) > -{ > - /* Reject invalid timeouts. */ > - if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) > - return EINVAL; > - > - do > - { > - 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. */ > - int oldval = atomic_compare_and_exchange_val_acq (futex, 2, 1); > - if (oldval != 0) > - lll_futex_timed_wait (futex, 2, &rt, private); > - } > - while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0); > - return 0; > -} > - > - > -/* These don't get included in libc.so */ > -#ifdef IS_IN_libpthread > -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. */ > - if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT) > - return ETIMEDOUT; > - } > - > - return 0; > -} > -#endif > - > Cheers, Carlos.
On 04/28/2014 11:51 AM, Bernie Ogden wrote: > lowlevellock.c for hppa differs from the generic lowlevellock.c only in > insignificant ways, so can be removed. I don't have any hppa targets > to work with, so have not been able to test this patch. > > The notable differences between the hppa and generic implementations are: > > 1) Some functions in hppa's lowlevellock.c set futex to 2 if it was 1. > The generic version always sets the futex to 2. As futex can only be > 0, 1 or 2 on entry into these functions, the behaviour is equivalent. > (If the futex manages to be 0 on entry then we've just lost an > unlikely fast path out.) > 2) hppa places most of the functions in this file in both libc and > libpthread. The generic implementation places only > __lll_lock_wait_private in libc. As these are internal functions I > think that, if this does cause a problem, it would show up at build > time - but I'm new to glibc so prepared to be corrected (and have not > been able to build). > > I would be grateful if the maintainer could test/comment. > > Regards, > > Bernie > > ports/ChangeLog.hppa > 2014-04-24 Bernard Ogden <bernie.ogden@linaro.org> > > [BZ #15119] > * ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c: Remove file. Looks good to me. hppa's futex implementation should match the generic defaults at all times. Cheers, Carlos.
I've completed failed to ping this - sorry. Would you mind committing? Thanks, Bernie On 29 Apr 2014, at 10:14, Carlos O'Donell <carlos@redhat.com> wrote: > On 04/28/2014 11:51 AM, Bernie Ogden wrote: >> lowlevellock.c for hppa differs from the generic lowlevellock.c only in >> insignificant ways, so can be removed. I don't have any hppa targets >> to work with, so have not been able to test this patch. >> >> The notable differences between the hppa and generic implementations are: >> >> 1) Some functions in hppa's lowlevellock.c set futex to 2 if it was 1. >> The generic version always sets the futex to 2. As futex can only be >> 0, 1 or 2 on entry into these functions, the behaviour is equivalent. >> (If the futex manages to be 0 on entry then we've just lost an >> unlikely fast path out.) >> 2) hppa places most of the functions in this file in both libc and >> libpthread. The generic implementation places only >> __lll_lock_wait_private in libc. As these are internal functions I >> think that, if this does cause a problem, it would show up at build >> time - but I'm new to glibc so prepared to be corrected (and have not >> been able to build). >> >> I would be grateful if the maintainer could test/comment. >> >> Regards, >> >> Bernie >> >> ports/ChangeLog.hppa >> 2014-04-24 Bernard Ogden <bernie.ogden@linaro.org> >> >> [BZ #15119] >> * ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c: Remove file. > > Looks good to me. > > hppa's futex implementation should match the generic defaults at all times. > > Cheers, > Carlos. >
On 06/09/2014 04:29 PM, Bernard Ogden wrote: > I've completed failed to ping this - sorry. > > Would you mind committing? Pushed. All discussions now happen on libc-alpha. This mailing list is legacy now. Cheers, Carlos.
diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c b/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c deleted file mode 100644 index d61c5d3..0000000 --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.c +++ /dev/null @@ -1,127 +0,0 @@ -/* low level locking for pthread library. Generic futex-using version. - Copyright (C) 2003-2014 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Paul Mackerras <paulus@au.ibm.com>, 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/>. */ - -#include <errno.h> -#include <sysdep.h> -#include <lowlevellock.h> -#include <sys/time.h> - -void -__lll_lock_wait (lll_lock_t *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_val_acq (futex, 2, 0) != 0); -} - -void -__lll_lock_wait_private (lll_lock_t *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_val_acq (futex, 2, 0) != 0); -} - -int -__lll_timedlock_wait (lll_lock_t *futex, const struct timespec *abstime, int private) -{ - /* Reject invalid timeouts. */ - if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) - return EINVAL; - - do - { - 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. */ - int oldval = atomic_compare_and_exchange_val_acq (futex, 2, 1); - if (oldval != 0) - lll_futex_timed_wait (futex, 2, &rt, private); - } - while (atomic_compare_and_exchange_bool_acq (futex, 2, 0) != 0); - return 0; -} - - -/* These don't get included in libc.so */ -#ifdef IS_IN_libpthread -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. */ - if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT) - return ETIMEDOUT; - } - - return 0; -} -#endif