[htl,v3,2/5] htl: Move thread table to ld.so
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
The next commit is going to introduce a new implementation of
THREAD_GSCOPE_WAIT which needs to access the list of threads.
Since it must be usable from the dynamic laoder, we have to move
the symbols for the list of threads into the loader.
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
htl/Versions | 2 -
htl/pt-alloc.c | 50 ++++++++++--------------
htl/pt-create.c | 11 ++----
htl/pt-internal.h | 23 ++++-------
sysdeps/generic/ldsodefs.h | 9 +++++
sysdeps/htl/dl-support.c | 23 +++++++++++
sysdeps/htl/pt-key-delete.c | 8 ++--
sysdeps/htl/pthreadP.h | 2 -
sysdeps/htl/raise.c | 8 +++-
sysdeps/htl/thrd_current.c | 7 +++-
sysdeps/mach/hurd/htl/pt-sigstate-init.c | 2 +-
sysdeps/mach/hurd/htl/pt-sysdep.c | 2 +-
sysdeps/mach/hurd/htl/pt-sysdep.h | 2 +-
13 files changed, 82 insertions(+), 67 deletions(-)
create mode 100644 sysdeps/htl/dl-support.c
Comments
Hello,
Sergey Bugaev, le mar. 07 sept. 2021 16:33:22 +0300, a ecrit:
> diff --git a/htl/Versions b/htl/Versions
> index 4aea321016..4e0ebac285 100644
> --- a/htl/Versions
> +++ b/htl/Versions
> @@ -132,25 +122,25 @@ __pthread_alloc (struct __pthread **pthread)
> }
>
> retry:
> - __pthread_rwlock_wrlock (&__pthread_threads_lock);
> + lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
Please rather use __libc_rwlock_wrlock etc.
Even if for now they are actually locks and not rwlocks, the day we fix
that the rwlocking of dl_pthread_threads_lock will get fixed alongside.
> - /* Store a pointer to this thread in the thread ID lookup table. We
> - could use __thread_setid, however, we only lock for reading as no
> - other thread should be using this entry (we also assume that the
> - store is atomic). */
> - __pthread_rwlock_rdlock (&__pthread_threads_lock);
> - __pthread_threads[pthread->thread - 1] = pthread;
> - __pthread_rwlock_unlock (&__pthread_threads_lock);
> + /* Store a pointer to this thread in the thread ID lookup table. */
> + __pthread_setid (pthread->thread, pthread);
We still want to optimize for read-lock.
Apart from this it looks good, thanks!
Samuel
On Wed, Sep 15, 2021 at 2:27 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Please rather use __libc_rwlock_wrlock etc.
Thanks, I did not realize there were actually more internal
synchronization primitives implemented on top of lll. I will send out
a new version shortly.
> Even if for now they are actually locks and not rwlocks, the day we fix
> that the rwlocking of dl_pthread_threads_lock will get fixed alongside.
May I ask, why are rwlocks unimplemented? Is it just that nobody has
done the work, or are there some unobvious complications?
I'm asking because I happen to have an implementation of rwlocks [0]
that works (passes the test) on the Hurd. It makes use of futex
bitsets (which don't seem to be supported by gsync), but seems to work
just fine without them. And HTL of course has its own rwlocks
implementation. Would it make sense to me to spend some time to try
and write a __libc_rwlock based on them, or are there some obstacles?
[0]: https://github.com/bugaevc/lets-write-sync-primitives/blob/master/src/rwlock.cpp
Sergey
Sergey Bugaev, le mer. 15 sept. 2021 17:14:00 +0300, a ecrit:
> > Even if for now they are actually locks and not rwlocks, the day we fix
> > that the rwlocking of dl_pthread_threads_lock will get fixed alongside.
>
> May I ask, why are rwlocks unimplemented? Is it just that nobody has
> done the work,
This, yes, as usual :)
> And HTL of course has its own rwlocks implementation. Would it make
> sense to me to spend some time to try and write a __libc_rwlock based
> on them, or are there some obstacles?
Well, at some point we'll probably want to just do like nptl and
simply #define __libc_rwlock pthread_rwlock, so it's probably not
worth spending time on making a separate __libc_rwlock implementation,
and rather spend it on making pthread_rwlock use gsync, like was done
for pthread_mutex and sem, and then we can just #define __libc_rwlock
pthread_rwlock.
Samuel
Samuel Thibault, le mer. 15 sept. 2021 16:34:27 +0200, a ecrit:
> making pthread_rwlock use gsync, like was done for pthread_mutex and
> sem, and then we can just #define __libc_rwlock pthread_rwlock.
I forgot an important point: that will also depend on moving pthread_*
from libpthread.so to libc.so, which we'll also want sooner or later.
Samuel
On Wed, Sep 15, 2021 at 5:34 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> so it's probably not
> worth spending time on making a separate __libc_rwlock implementation,
> and rather spend it on making pthread_rwlock use gsync, like was done
> for pthread_mutex and sem
Oh, it currently doesn't? That's horrifying. Then I'll look into that next.
Sergey
P.S. There's this other thing that is way more urgent than either
eliminating gsync_wake () calls or optimizing rwlocks. I sent you an
email a week ago. Please take a look.
Sergey Bugaev, le mer. 15 sept. 2021 18:13:15 +0300, a ecrit:
> P.S. There's this other thing that is way more urgent than either
> eliminating gsync_wake () calls or optimizing rwlocks. I sent you an
> email a week ago. Please take a look.
I know.
My mbox is still 700 mails long.
Samuel
@@ -168,8 +168,6 @@ libpthread {
GLIBC_PRIVATE {
__pthread_initialize_minimal;
- __pthread_threads;
-
__cthread_detach;
__cthread_fork;
__pthread_detach;
@@ -28,19 +28,9 @@
of the threads functions "shall fail" if "No thread could be found
corresponding to that specified by the given thread ID." */
-/* Thread ID lookup table. */
-struct __pthread **__pthread_threads;
-
/* The size of the thread ID lookup table. */
int __pthread_max_threads;
-/* The total number of thread IDs currently in use, or on the list of
- available thread IDs. */
-int __pthread_num_threads;
-
-/* A lock for the table, and the other variables above. */
-pthread_rwlock_t __pthread_threads_lock;
-
/* List of thread structures corresponding to free thread IDs. */
struct __pthread *__pthread_free_threads;
pthread_mutex_t __pthread_free_threads_lock;
@@ -132,25 +122,25 @@ __pthread_alloc (struct __pthread **pthread)
}
retry:
- __pthread_rwlock_wrlock (&__pthread_threads_lock);
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
- if (__pthread_num_threads < __pthread_max_threads)
+ if (GL (dl_pthread_num_threads) < __pthread_max_threads)
{
/* We have a free slot. Use the slot number plus one as the
thread ID for the new thread. */
- new->thread = 1 + __pthread_num_threads++;
- __pthread_threads[new->thread - 1] = NULL;
+ new->thread = 1 + GL (dl_pthread_num_threads)++;
+ GL (dl_pthread_threads)[new->thread - 1] = NULL;
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
*pthread = new;
return 0;
}
#ifdef PTHREAD_THREADS_MAX
- else if (__pthread_num_threads >= PTHREAD_THREADS_MAX)
+ else if (GL (dl_pthread_num_threads) >= PTHREAD_THREADS_MAX)
{
/* We have reached the limit on the number of threads per process. */
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
free (new);
return EAGAIN;
@@ -162,7 +152,7 @@ retry:
memory allocation, since that's a potentially blocking operation. */
max_threads = __pthread_max_threads;
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
/* Allocate a new lookup table that's twice as large. */
new_max_threads
@@ -174,13 +164,13 @@ retry:
return ENOMEM;
}
- __pthread_rwlock_wrlock (&__pthread_threads_lock);
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
/* Check if nobody else has already enlarged the table. */
if (max_threads != __pthread_max_threads)
{
/* Yep, they did. */
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
/* Free the newly allocated table and try again to allocate a slot. */
free (threads);
@@ -188,22 +178,22 @@ retry:
}
/* Copy over the contents of the old table. */
- memcpy (threads, __pthread_threads,
+ memcpy (threads, GL (dl_pthread_threads),
__pthread_max_threads * sizeof (struct __pthread *));
/* Save the location of the old table. We want to deallocate its
storage after we released the lock. */
- old_threads = __pthread_threads;
+ old_threads = GL (dl_pthread_threads);
/* Replace the table with the new one. */
__pthread_max_threads = new_max_threads;
- __pthread_threads = threads;
+ GL (dl_pthread_threads) = threads;
/* And allocate ourselves one of the newly created slots. */
- new->thread = 1 + __pthread_num_threads++;
- __pthread_threads[new->thread - 1] = NULL;
+ new->thread = 1 + GL (dl_pthread_num_threads)++;
+ GL (dl_pthread_threads)[new->thread - 1] = NULL;
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
free (old_threads);
@@ -217,10 +207,10 @@ __pthread_init_static_tls (struct link_map *map)
{
int i;
- __pthread_rwlock_wrlock (&__pthread_threads_lock);
- for (i = 0; i < __pthread_num_threads; ++i)
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
+ for (i = 0; i < GL (dl_pthread_num_threads); ++i)
{
- struct __pthread *t = __pthread_threads[i];
+ struct __pthread *t = GL (dl_pthread_threads)[i];
if (t == NULL)
continue;
@@ -237,5 +227,5 @@ __pthread_init_static_tls (struct link_map *map)
memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
'\0', map->l_tls_blocksize - map->l_tls_initimage_size);
}
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
}
@@ -207,7 +207,7 @@ __pthread_create_internal (struct __pthread **thread,
creating thread. The set of signals pending for the new thread
shall be empty." If the currnet thread is not a pthread then we
just inherit the process' sigmask. */
- if (__pthread_num_threads == 1)
+ if (GL (dl_pthread_num_threads) == 1)
err = __sigprocmask (0, 0, &pthread->init_sigset);
else
err = __pthread_sigstate (_pthread_self (), 0, 0, &pthread->init_sigset, 0);
@@ -227,13 +227,8 @@ __pthread_create_internal (struct __pthread **thread,
new thread runs. */
atomic_increment (&__pthread_total);
- /* Store a pointer to this thread in the thread ID lookup table. We
- could use __thread_setid, however, we only lock for reading as no
- other thread should be using this entry (we also assume that the
- store is atomic). */
- __pthread_rwlock_rdlock (&__pthread_threads_lock);
- __pthread_threads[pthread->thread - 1] = pthread;
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ /* Store a pointer to this thread in the thread ID lookup table. */
+ __pthread_setid (pthread->thread, pthread);
/* At this point it is possible to guess our pthread ID. We have to
make sure that all functions taking a pthread_t argument can
@@ -166,33 +166,24 @@ __pthread_dequeue (struct __pthread *thread)
/* The total number of threads currently active. */
extern unsigned int __pthread_total;
-/* The total number of thread IDs currently in use, or on the list of
- available thread IDs. */
-extern int __pthread_num_threads;
-
/* Concurrency hint. */
extern int __pthread_concurrency;
-/* Array of __pthread structures and its lock. Indexed by the pthread
- id minus one. (Why not just use the pthread id? Because some
- brain-dead users of the pthread interface incorrectly assume that 0
- is an invalid pthread id.) */
-extern struct __pthread **__pthread_threads;
+/* The size of the thread ID lookup table. */
extern int __pthread_max_threads;
-extern pthread_rwlock_t __pthread_threads_lock;
#define __pthread_getid(thread) \
({ struct __pthread *__t = NULL; \
- __pthread_rwlock_rdlock (&__pthread_threads_lock); \
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE); \
if (thread <= __pthread_max_threads) \
- __t = __pthread_threads[thread - 1]; \
- __pthread_rwlock_unlock (&__pthread_threads_lock); \
+ __t = GL (dl_pthread_threads)[thread - 1]; \
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE); \
__t; })
#define __pthread_setid(thread, pthread) \
- __pthread_rwlock_wrlock (&__pthread_threads_lock); \
- __pthread_threads[thread - 1] = pthread; \
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE); \
+ GL (dl_pthread_threads)[thread - 1] = pthread; \
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
/* Similar to pthread_self, but returns the thread descriptor instead
of the thread ID. */
@@ -486,7 +486,16 @@ struct rtld_global
/* Mutex protecting the stack lists. */
EXTERN int _dl_stack_cache_lock;
+#else
+ /* The total number of thread IDs currently in use, or on the list of
+ available thread IDs. */
+ EXTERN int _dl_pthread_num_threads;
+
+ /* Array of __pthread structures and its lock. */
+ EXTERN struct __pthread **_dl_pthread_threads;
+ EXTERN int _dl_pthread_threads_lock;
#endif
+
#if !THREAD_GSCOPE_IN_TCB
EXTERN int _dl_thread_gscope_count;
#endif
new file mode 100644
@@ -0,0 +1,23 @@
+/* Support for dynamic linking code in static libc.
+ Copyright (C) 2007-2021 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <elf/dl-support.c>
+
+int _dl_pthread_num_threads;
+struct __pthread **_dl_pthread_threads;
+int _dl_pthread_threads_lock;
@@ -39,12 +39,12 @@ __pthread_key_delete (pthread_key_t key)
__pthread_key_destructors[key] = PTHREAD_KEY_INVALID;
__pthread_key_invalid_count++;
- __pthread_rwlock_rdlock (&__pthread_threads_lock);
- for (i = 0; i < __pthread_num_threads; ++i)
+ lll_lock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
+ for (i = 0; i < GL (dl_pthread_num_threads); ++i)
{
struct __pthread *t;
- t = __pthread_threads[i];
+ t = GL (dl_pthread_threads)[i];
if (t == NULL)
continue;
@@ -54,7 +54,7 @@ __pthread_key_delete (pthread_key_t key)
if (key < t->thread_specifics_size)
t->thread_specifics[key] = 0;
}
- __pthread_rwlock_unlock (&__pthread_threads_lock);
+ lll_unlock (GL (dl_pthread_threads_lock), LLL_PRIVATE);
}
__pthread_mutex_unlock (&__pthread_key_lock);
@@ -31,8 +31,6 @@ extern void __pthread_init_static_tls (struct link_map *) attribute_hidden;
/* These represent the interface used by glibc itself. */
-extern struct __pthread **__pthread_threads;
-
extern int __pthread_mutex_init (pthread_mutex_t *__mutex, const pthread_mutexattr_t *__attr);
extern int __pthread_mutex_destroy (pthread_mutex_t *__mutex);
extern int __pthread_mutex_lock (pthread_mutex_t *__mutex);
@@ -17,6 +17,7 @@
License along with this program. If not, see
<https://www.gnu.org/licenses/>. */
+#include <ldsodefs.h>
#include <pthreadP.h>
#include <signal.h>
#include <unistd.h>
@@ -24,6 +25,11 @@
#pragma weak __pthread_kill
#pragma weak __pthread_self
#pragma weak __pthread_threads
+
+#ifndef SHARED
+#pragma weak _dl_pthread_threads
+#endif
+
int
raise (int signo)
{
@@ -31,7 +37,7 @@ raise (int signo)
"the effect of the raise() function shall be equivalent to
calling: pthread_kill(pthread_self(), sig);" */
- if (__pthread_kill != NULL && __pthread_threads != NULL)
+ if (__pthread_kill != NULL && GL (dl_pthread_threads) != NULL)
{
int err;
err = __pthread_kill (__pthread_self (), signo);
@@ -17,14 +17,19 @@
<https://www.gnu.org/licenses/>. */
#include "thrd_priv.h"
+#include <ldsodefs.h>
#pragma weak __pthread_self
#pragma weak __pthread_threads
+#ifndef SHARED
+#pragma weak _dl_pthread_threads
+#endif
+
thrd_t
thrd_current (void)
{
- if (__pthread_threads)
+ if (GL (dl_pthread_threads))
return (thrd_t) __pthread_self ();
return (thrd_t) 0;
@@ -37,7 +37,7 @@ __pthread_sigstate_init (struct __pthread *thread)
struct hurd_sigstate *ss = _hurd_thread_sigstate (thread->kernel_thread);
_hurd_sigstate_set_global_rcv (ss);
}
- else if (__pthread_num_threads >= 2)
+ else if (GL (dl_pthread_num_threads) >= 2)
do_init_global = 1;
return 0;
@@ -45,7 +45,7 @@ _init_routine (void *stack)
int err;
pthread_attr_t attr, *attrp = 0;
- if (__pthread_threads != NULL)
+ if (GL (dl_pthread_threads) != NULL)
/* Already initialized */
return;
@@ -37,7 +37,7 @@ extern __thread struct __pthread *___pthread_self;
({ \
struct __pthread *thread; \
\
- assert (__pthread_threads); \
+ assert (GL (dl_pthread_threads)); \
thread = ___pthread_self; \
\
assert (thread); \