[RFC,htl,v2,0/4] Rewrite THREAD_GSCOPE

Message ID 20210830152636.1334678-1-bugaevc@gmail.com
Headers
Series Rewrite THREAD_GSCOPE |

Message

Sergey Bugaev Aug. 30, 2021, 3:26 p.m. UTC
  Hello,

this is a revised version of the patchset I have sent a few months ago [0].
Sending to both bug-hurd and libc-alpha this time.

[0]: https://lists.gnu.org/archive/html/bug-hurd/2021-05/msg00063.html

The goal is to get rid of the excessive (and redundant) gsync_wake () calls.
The means are rewriting how THREAD_GSCOPE_* is implemented in HTL to work more
like its NPTL counterpart. To get this working, I first have to expose HTL's
internal thread table to ld.so, a lot like NPTL already does.

Before this patchset:
$ rpctrace uname |& grep -c gsync_wake
57

With this patchset (first 3 patches):
$ ./testrun.sh --tool=rpctrace /bin/uname |& grep -c gsync_wake
0

The unsolved question with this patchset is the fate of THREAD_GSCOPE_IN_TCB.
Apparently, when it was introduced in [1], it was indeed only used to
distinguish between cases where the GSCOPE flag is in TCB or not. But as of
now, it is used mostly as a generic way to conditionally compile either HTL-
or NPTL-specific code, mostly for TLS initialization. Now that all ports have
the GSCOPE flag in TCB, this definition no longer does what it says.

[1]: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=a5df0318ef30a4dcff3fa2cb82265f641813d9ea

I can think of two ways this can be handled:

* Code can be rewritten and reorganized to use port-specific files instead of
  conditional compilation. From looking at how glibc source is organized, this
  appears to be the generally preferred way. I'm sending one more patch that
  makes a (perhaps misguided) attempt to do this; but I'm not sure what would
  be good way to handle the definitions in <ldsodefs.h> -- please take a look.

* A more conservative approach would be to leave the code using
  THREAD_GSCOPE_IN_TCB as is for now, but change the variable's name to better
  reflect what it's used for. I haven't really been able to come up with a good
  name, though.

Alternatively, it could just be left as is for now.

Sergey Bugaev (4):
  htl: Move thread table to ld.so
  htl: Reimplement GSCOPE
  testrun.sh: Add support for --tool=rpctrace
  XXX: Attempt to get rid of most THREAD_GSCOPE_IN_TCB usages

 Makefile                                   |  9 +++-
 elf/dl-reloc.c                             | 21 ---------
 elf/dl-support.c                           | 15 ------
 elf/dl-tls.c                               | 39 ---------------
 htl/Versions                               |  2 -
 htl/pt-alloc.c                             | 50 ++++++++------------
 htl/pt-create.c                            | 11 ++---
 htl/pt-internal.h                          | 23 +++------
 sysdeps/generic/ldsodefs.h                 | 10 ++--
 sysdeps/htl/dl-support.c                   | 28 +++++++++++
 sysdeps/htl/dl-thread_gscope_wait.c        | 55 ++++++++++++++++++++++
 sysdeps/htl/dl-tls.c                       | 39 +++++++++++++++
 {elf => sysdeps/htl}/dl-tls_init_tp.c      |  4 +-
 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 +-
 sysdeps/mach/hurd/i386/tls.h               | 19 ++++++++
 sysdeps/mach/hurd/tls.h                    | 20 +-------
 sysdeps/nptl/dl-support.c                  | 26 ++++++++++
 sysdeps/nptl/dl-tls.c                      | 53 +++++++++++++++++++++
 sysdeps/unix/sysv/linux/alpha/dl-support.c |  2 +-
 25 files changed, 287 insertions(+), 170 deletions(-)
 create mode 100644 sysdeps/htl/dl-support.c
 create mode 100644 sysdeps/htl/dl-thread_gscope_wait.c
 create mode 100644 sysdeps/htl/dl-tls.c
 rename {elf => sysdeps/htl}/dl-tls_init_tp.c (93%)
 create mode 100644 sysdeps/nptl/dl-support.c
 create mode 100644 sysdeps/nptl/dl-tls.c
  

Comments

Florian Weimer Aug. 30, 2021, 3:37 p.m. UTC | #1
* Sergey Bugaev:

> I can think of two ways this can be handled:
>
> * Code can be rewritten and reorganized to use port-specific files instead of
>   conditional compilation. From looking at how glibc source is organized, this
>   appears to be the generally preferred way. I'm sending one more patch that
>   makes a (perhaps misguided) attempt to do this; but I'm not sure what would
>   be good way to handle the definitions in <ldsodefs.h> -- please take a look.

Yes, that's usually how things are done.

> * A more conservative approach would be to leave the code using
>   THREAD_GSCOPE_IN_TCB as is for now, but change the variable's name to
>   better reflect what it's used for. I haven't really been able to come
>   up with a good name, though.

You could kick the can down the road by switching to PTHREAD_IN_LIBC.
In some cases, that might actually be correct replacement.

Thanks,
Florian
  
Sergey Bugaev Aug. 31, 2021, 6:16 p.m. UTC | #2
Thanks for looking into it!

On Mon, Aug 30, 2021 at 6:37 PM Florian Weimer <fweimer@redhat.com> wrote:
> You could kick the can down the road by switching to PTHREAD_IN_LIBC.
> In some cases, that might actually be correct replacement.

Ack; that should be easy to do. Nevertheless, please do look at patch
4/4 and say if you would prefer that approach.

Do you happen to know if there's a testcase that exercises
THREAD_GSCOPE_WAIT ()? Even simple programs evidently use
THREAD_GSCOPE_SET_FLAG () / THREAD_GSCOPE_RESET_FLAG (), but I so far
haven't been able to construct a program that would trigger the WAIT.
I've tried dlopening and dlclosing libraries concurrently from several
threads; I will try dlopening hundreds of libraries next (to hopefully
trigger a reallocation of some table), but it would be handy if there
was an official ready-to-use testcase.

Sergey