mbox series

[v3,0/6] Add support for ISO C11 threads.h

Message ID 1490103612-9401-1-git-send-email-adhemerval.zanella@linaro.org
Headers show
Series Add support for ISO C11 threads.h | expand

Message

Adhemerval Zanella Netto March 21, 2017, 1:40 p.m. UTC
This is a new update of C11 thread support based on version 2 from 
Juan Manuel Torres Palma [1].  I contact him to take over his initial
patches and extended to fix some issues and update to glibc 2.26.

The main changes are:

  - Addresses Joseph's comments from [2] and ad indicated in
    in C11 thread patch I based this implementation from th
    public C11 thread document I have access [3]. I would ask for
    suggestion and change if it deviates from the final standard
    definition.

  - Fix remaining namespace and linkspace issues by building on
    basically all supported architectures (aarch64, alpha, armhf,
    hppa, i686, ia64, m68k, microblaze, mips64, mips64n32, mips,
    nios2, powerpc64le, powerpc64, powerpc, powerpc-power4, s390,
    s390x, sh4, sparc64, sparcv9, tilegx, tilegx32, tilepro,
    x86_64, and x86_64-32).  Note: hppa changes still results in
    broken build as indicated in BZ#21016.

  - Adjusted abi files to 2.26, headers dates to 2017 and some
    others typos.

  - Fix a calling convention on pthread_create to call c11 start
    function with a well-behaved semanthics.

I tested this without regression with a full make/check on
x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
powerpc-linux-gnu, aarch64-linux-gnu, arm-linux-gnueabihf,
sparc64-linux-gnu, and sparcv9-linux-gnu.

[1] https://sourceware.org/ml/libc-alpha/2015-08/msg01051.html
[2] https://sourceware.org/ml/libc-alpha/2016-03/msg00604.html
[3] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf

Adhemerval Zanella (5):
  Consolidate pthreadtype.h placement
  Move mutex and condition variable definition to common header
  Clean pthread functions namespaces for C11 threads
  Add C11 threads support
  Add test cases for ISO C11 threads

Juan Manuel Torres Palma (1):
  Add manual documentation for threads.h

 ChangeLog                                          | 247 ++++++++++++++
 bits/thread-shared-types.h                         |  36 ++
 conform/Makefile                                   |   7 +-
 conform/data/threads.h-data                        |  56 ++++
 include/sched.h                                    |   2 +
 include/stdc-predef.h                              |   3 -
 include/sys/mman.h                                 |   4 +
 include/termios.h                                  |   2 +
 include/time.h                                     |   2 +-
 manual/Makefile                                    |   2 +-
 manual/isothreads.texi                             | 373 +++++++++++++++++++++
 misc/Versions                                      |   2 +
 nptl/Makefile                                      |  17 +-
 nptl/Versions                                      |  16 +
 nptl/allocatestack.c                               |  24 +-
 nptl/call_once.c                                   |  27 ++
 nptl/cnd_broadcast.c                               |  28 ++
 nptl/cnd_destroy.c                                 |  28 ++
 nptl/cnd_init.c                                    |  27 ++
 nptl/cnd_signal.c                                  |  28 ++
 nptl/cnd_timedwait.c                               |  32 ++
 nptl/cnd_wait.c                                    |  28 ++
 nptl/descr.h                                       |   1 +
 nptl/mtx_destroy.c                                 |  27 ++
 nptl/mtx_init.c                                    |  48 +++
 nptl/mtx_lock.c                                    |  29 ++
 nptl/mtx_timedlock.c                               |  31 ++
 nptl/mtx_trylock.c                                 |  29 ++
 nptl/mtx_unlock.c                                  |  28 ++
 nptl/pthreadP.h                                    |  12 +
 nptl/pthread_cancel.c                              |   7 +-
 nptl/pthread_create.c                              |  39 ++-
 nptl/pthread_detach.c                              |   3 +-
 nptl/pthread_equal.c                               |   2 +-
 nptl/pthread_exit.c                                |   4 +-
 nptl/pthread_getspecific.c                         |   2 +-
 nptl/pthread_join.c                                |   3 +-
 nptl/pthread_key_create.c                          |   2 +-
 nptl/pthread_key_delete.c                          |   3 +-
 nptl/pthread_mutex_destroy.c                       |   2 +-
 nptl/pthread_mutex_init.c                          |   2 +-
 nptl/pthread_mutex_lock.c                          |   2 +-
 nptl/pthread_mutex_timedlock.c                     |   5 +-
 nptl/pthread_mutex_trylock.c                       |   3 +-
 nptl/pthread_mutex_unlock.c                        |   2 +-
 nptl/pthread_mutexattr_init.c                      |   3 +-
 nptl/pthread_mutexattr_settype.c                   |   3 +-
 nptl/pthread_self.c                                |   2 +-
 nptl/pthread_setspecific.c                         |   2 +-
 nptl/thrd_create.c                                 |  30 ++
 nptl/thrd_current.c                                |  26 ++
 nptl/thrd_detach.c                                 |  30 ++
 nptl/thrd_equal.c                                  |  26 ++
 nptl/thrd_exit.c                                   |  27 ++
 nptl/thrd_join.c                                   |  32 ++
 nptl/thrd_priv.h                                   |  46 +++
 nptl/thrd_sleep.c                                  |  41 +++
 nptl/thrd_yield.c                                  |  29 ++
 nptl/tpp.c                                         |   4 +-
 nptl/tss_create.c                                  |  29 ++
 nptl/tss_delete.c                                  |  27 ++
 nptl/tss_get.c                                     |  27 ++
 nptl/tss_set.c                                     |  28 ++
 nptl/tst-call-once.c                               |  66 ++++
 nptl/tst-cnd-basic.c                               |  68 ++++
 nptl/tst-cnd-broadcast.c                           |  83 +++++
 nptl/tst-cnd-timedwait.c                           |  70 ++++
 nptl/tst-mtx-basic.c                               |  73 ++++
 nptl/tst-mtx-recursive.c                           |  45 +++
 nptl/tst-mtx-timedlock.c                           |  98 ++++++
 nptl/tst-mtx-trylock.c                             |  90 +++++
 nptl/tst-thrd-detach.c                             |  52 +++
 nptl/tst-thrd-sleep.c                              |  51 +++
 nptl/tst-tss-basic.c                               |  75 +++++
 posix/Makefile                                     |   3 +-
 posix/nanosleep.c                                  |   2 +-
 sysdeps/aarch64/nptl/bits/pthreadtypes.h           |  67 +---
 sysdeps/aarch64/nptl/bits/thread-shared-types.h    |  97 ++++++
 .../linux/alpha => alpha/nptl}/bits/pthreadtypes.h |  63 +---
 sysdeps/alpha/nptl/bits/thread-shared-types.h      |  93 +++++
 sysdeps/arm/nptl/bits/pthreadtypes.h               |  67 +---
 sysdeps/arm/nptl/bits/thread-shared-types.h        |  95 ++++++
 sysdeps/hppa/nptl/bits/pthreadtypes.h              |  83 +----
 sysdeps/hppa/nptl/bits/thread-shared-types.h       | 105 ++++++
 sysdeps/i386/nptl/tls.h                            |   2 +-
 sysdeps/ia64/nptl/bits/pthreadtypes.h              |  67 +---
 sysdeps/ia64/nptl/bits/thread-shared-types.h       |  95 ++++++
 sysdeps/m68k/nptl/bits/pthreadtypes.h              |  68 +---
 sysdeps/m68k/nptl/bits/thread-shared-types.h       | 100 ++++++
 sysdeps/microblaze/nptl/bits/pthreadtypes.h        |  61 +---
 sysdeps/microblaze/nptl/bits/thread-shared-types.h |  96 ++++++
 sysdeps/mips/nptl/bits/pthreadtypes.h              |  83 +----
 sysdeps/mips/nptl/bits/thread-shared-types.h       | 117 +++++++
 sysdeps/nacl/libpthread.abilist                    |  26 ++
 sysdeps/nios2/nptl/bits/pthreadtypes.h             |  67 +---
 sysdeps/nios2/nptl/bits/thread-shared-types.h      |  93 +++++
 sysdeps/nptl/bits/pthreadtypes-common.h            |  44 +++
 sysdeps/nptl/threads.h                             | 198 +++++++++++
 sysdeps/posix/gethostname.c                        |   2 +-
 .../powerpc => powerpc/nptl}/bits/pthreadtypes.h   | 100 +-----
 sysdeps/powerpc/nptl/bits/thread-shared-types.h    | 121 +++++++
 sysdeps/s390/nptl/bits/pthreadtypes.h              | 103 +-----
 sysdeps/s390/nptl/bits/thread-shared-types.h       | 140 ++++++++
 sysdeps/sh/nptl/bits/pthreadtypes.h                |  63 +---
 sysdeps/sh/nptl/bits/thread-shared-types.h         |  98 ++++++
 sysdeps/sparc/nptl/bits/pthreadtypes.h             |  82 +----
 sysdeps/sparc/nptl/bits/thread-shared-types.h      | 119 +++++++
 sysdeps/tile/nptl/bits/pthreadtypes.h              |  95 +-----
 sysdeps/tile/nptl/bits/thread-shared-types.h       | 118 +++++++
 sysdeps/unix/sysv/linux/aarch64/libpthread.abilist |  26 ++
 sysdeps/unix/sysv/linux/aarch64/mmap.c             |   5 +-
 sysdeps/unix/sysv/linux/alpha/libpthread.abilist   |  26 ++
 sysdeps/unix/sysv/linux/arm/libpthread.abilist     |  26 ++
 sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c |   5 +-
 sysdeps/unix/sysv/linux/hppa/libpthread.abilist    |  26 ++
 sysdeps/unix/sysv/linux/hppa/mmap.c                |   1 +
 sysdeps/unix/sysv/linux/i386/Versions              |   2 +-
 sysdeps/unix/sysv/linux/i386/libpthread.abilist    |  26 ++
 sysdeps/unix/sysv/linux/i386/mmap.c                |   1 +
 sysdeps/unix/sysv/linux/i386/smp.h                 |   2 +-
 sysdeps/unix/sysv/linux/ia64/libpthread.abilist    |  26 ++
 .../sysv/linux/m68k/coldfire/libpthread.abilist    |  26 ++
 .../unix/sysv/linux/m68k/m680x0/libpthread.abilist |  26 ++
 sysdeps/unix/sysv/linux/m68k/mmap.S                |   1 +
 .../unix/sysv/linux/microblaze/libpthread.abilist  |  26 ++
 sysdeps/unix/sysv/linux/microblaze/mmap.S          |   1 +
 .../unix/sysv/linux/mips/mips32/libpthread.abilist |  26 ++
 .../unix/sysv/linux/mips/mips64/libpthread.abilist |  26 ++
 sysdeps/unix/sysv/linux/mips/mips64/n32/mmap.c     |   5 +-
 sysdeps/unix/sysv/linux/mmap64.c                   |   6 +-
 sysdeps/unix/sysv/linux/nios2/libpthread.abilist   |  26 ++
 sysdeps/unix/sysv/linux/powerpc/ioctl.c            |   6 +-
 .../linux/powerpc/powerpc32/libpthread.abilist     |  26 ++
 .../linux/powerpc/powerpc64/libpthread-le.abilist  |  26 ++
 .../linux/powerpc/powerpc64/libpthread.abilist     |  26 ++
 .../sysv/linux/s390/s390-32/libpthread.abilist     |  26 ++
 sysdeps/unix/sysv/linux/s390/s390-32/mmap.S        |   1 +
 sysdeps/unix/sysv/linux/s390/s390-32/mmap64.S      |   1 +
 .../sysv/linux/s390/s390-64/libpthread.abilist     |  26 ++
 sysdeps/unix/sysv/linux/s390/s390-64/mmap.S        |   2 +
 sysdeps/unix/sysv/linux/sh/libpthread.abilist      |  26 ++
 .../sysv/linux/sparc/sparc32/libpthread.abilist    |  26 ++
 .../sysv/linux/sparc/sparc64/libpthread.abilist    |  26 ++
 sysdeps/unix/sysv/linux/tcsetattr.c                |   3 +-
 .../linux/tile/tilegx/tilegx32/libpthread.abilist  |  26 ++
 .../linux/tile/tilegx/tilegx64/libpthread.abilist  |  26 ++
 .../sysv/linux/tile/tilepro/libpthread.abilist     |  26 ++
 sysdeps/unix/sysv/linux/wordsize-64/mmap.c         |   6 +-
 sysdeps/unix/sysv/linux/x86/Implies                |   1 +
 .../unix/sysv/linux/x86_64/64/libpthread.abilist   |  26 ++
 .../unix/sysv/linux/x86_64/x32/libpthread.abilist  |  26 ++
 sysdeps/x86/{ => nptl}/bits/pthreadtypes.h         | 104 +-----
 sysdeps/x86/nptl/bits/thread-shared-types.h        | 120 +++++++
 sysdeps/x86_64/nptl/tls.h                          |   6 +-
 sysdeps/x86_64/x32/nptl/tls.h                      |   2 +-
 155 files changed, 4998 insertions(+), 1226 deletions(-)
 create mode 100644 bits/thread-shared-types.h
 create mode 100644 conform/data/threads.h-data
 create mode 100644 manual/isothreads.texi
 create mode 100644 nptl/call_once.c
 create mode 100644 nptl/cnd_broadcast.c
 create mode 100644 nptl/cnd_destroy.c
 create mode 100644 nptl/cnd_init.c
 create mode 100644 nptl/cnd_signal.c
 create mode 100644 nptl/cnd_timedwait.c
 create mode 100644 nptl/cnd_wait.c
 create mode 100644 nptl/mtx_destroy.c
 create mode 100644 nptl/mtx_init.c
 create mode 100644 nptl/mtx_lock.c
 create mode 100644 nptl/mtx_timedlock.c
 create mode 100644 nptl/mtx_trylock.c
 create mode 100644 nptl/mtx_unlock.c
 create mode 100644 nptl/thrd_create.c
 create mode 100644 nptl/thrd_current.c
 create mode 100644 nptl/thrd_detach.c
 create mode 100644 nptl/thrd_equal.c
 create mode 100644 nptl/thrd_exit.c
 create mode 100644 nptl/thrd_join.c
 create mode 100644 nptl/thrd_priv.h
 create mode 100644 nptl/thrd_sleep.c
 create mode 100644 nptl/thrd_yield.c
 create mode 100644 nptl/tss_create.c
 create mode 100644 nptl/tss_delete.c
 create mode 100644 nptl/tss_get.c
 create mode 100644 nptl/tss_set.c
 create mode 100644 nptl/tst-call-once.c
 create mode 100644 nptl/tst-cnd-basic.c
 create mode 100644 nptl/tst-cnd-broadcast.c
 create mode 100644 nptl/tst-cnd-timedwait.c
 create mode 100644 nptl/tst-mtx-basic.c
 create mode 100644 nptl/tst-mtx-recursive.c
 create mode 100644 nptl/tst-mtx-timedlock.c
 create mode 100644 nptl/tst-mtx-trylock.c
 create mode 100644 nptl/tst-thrd-detach.c
 create mode 100644 nptl/tst-thrd-sleep.c
 create mode 100644 nptl/tst-tss-basic.c
 create mode 100644 sysdeps/aarch64/nptl/bits/thread-shared-types.h
 rename sysdeps/{unix/sysv/linux/alpha => alpha/nptl}/bits/pthreadtypes.h (70%)
 create mode 100644 sysdeps/alpha/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/arm/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/hppa/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/ia64/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/m68k/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/microblaze/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/mips/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/nios2/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/nptl/bits/pthreadtypes-common.h
 create mode 100644 sysdeps/nptl/threads.h
 rename sysdeps/{unix/sysv/linux/powerpc => powerpc/nptl}/bits/pthreadtypes.h (63%)
 create mode 100644 sysdeps/powerpc/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/s390/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/sh/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/sparc/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/tile/nptl/bits/thread-shared-types.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/Implies
 rename sysdeps/x86/{ => nptl}/bits/pthreadtypes.h (64%)
 create mode 100644 sysdeps/x86/nptl/bits/thread-shared-types.h

-- 
2.7.4

Comments

Joseph Myers March 21, 2017, 4:25 p.m. UTC | #1
On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

>   - Addresses Joseph's comments from [2] and ad indicated in

>     in C11 thread patch I based this implementation from th

>     public C11 thread document I have access [3]. I would ask for

>     suggestion and change if it deviates from the final standard

>     definition.


Did you review those C11 DRs related to threads to make sure the 
implementation behaves properly regarding issues raised in those DRs (and 
that there are tests of this if applicable)?

Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Netto March 21, 2017, 4:43 p.m. UTC | #2
On 21/03/2017 13:25, Joseph Myers wrote:
> On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

> 

>>   - Addresses Joseph's comments from [2] and ad indicated in

>>     in C11 thread patch I based this implementation from th

>>     public C11 thread document I have access [3]. I would ask for

>>     suggestion and change if it deviates from the final standard

>>     definition.

> 

> Did you review those C11 DRs related to threads to make sure the 

> implementation behaves properly regarding issues raised in those DRs (and 

> that there are tests of this if applicable)?

> 

> Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

> 


Thanks for the link, I will check this out. Do you know off the top of your 
head which ones would be applicable?
Joseph Myers March 21, 2017, 4:49 p.m. UTC | #3
On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

> > Did you review those C11 DRs related to threads to make sure the 

> > implementation behaves properly regarding issues raised in those DRs (and 

> > that there are tests of this if applicable)?

> > 

> > Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

> > 

> 

> Thanks for the link, I will check this out. Do you know off the top of your 

> head which ones would be applicable? 


It appears to be: 405 414 416 424 449 469 470 479 480 493.

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Netto March 21, 2017, 5:08 p.m. UTC | #4
On 21/03/2017 13:49, Joseph Myers wrote:
> It appears to be: 405 414 416 424 449 469 470 479 480 493.


Thank you.
Adhemerval Zanella Netto March 21, 2017, 8:09 p.m. UTC | #5
On 21/03/2017 13:49, Joseph Myers wrote:
> On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

> 

>>> Did you review those C11 DRs related to threads to make sure the 

>>> implementation behaves properly regarding issues raised in those DRs (and 

>>> that there are tests of this if applicable)?

>>>

>>> Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

>>>

>>

>> Thanks for the link, I will check this out. Do you know off the top of your 

>> head which ones would be applicable? 

> 

> It appears to be: 405 414 416 424 449 469 470 479 480 493.


It indeed seems to be one related to threads definitions and in general
C11 clarification seems to align with POSIX thread definition, which
simplifies its implementation.  However I see some problematic ones:
416 (tss_t destruction unspecified) and 493 (Mutex Initialization
Underspecified), so I am copying Tovarld's and Martin since they were the
one that raised most the DR.  It could be good if they is any impending
reason to avoid implement C11 thread over POSIX primitives based
on current standard.

Below some observation of the raised DRs related to current patch
proposal:

405: The mutex specification

"The C11 specification of mutexes is missing the total order over all the
 calls on a particular mutex."

My understanding is current implementation with POSIX primitives should be
enough to guarantee C11 total order over mutex (which is currently based on
acquire/release semanthics).


416: tss_t destruction unspecified

I could not find any impeding wording from the proposed technical corrigendum
that would prevent us to implement tss function on top of pthread key ones.


414: Typos in 6.27 Threads <threads.h> 

The typos does clarify one definition of mtx_plain and raises a questioning: 
how should we proceed if mtx_timedlock is used along with a mtx_plain mutex? 

My understanding is thrd_error should be returned, however current GLIBC 
implementation makes no distinction between PTHREAD_MUTEX_NORMAL and 
PTHREAD_MUTEX_TIMED_NP (and current C11 patch follows this semanthic).

It is feasible to do so, but it would require maybe an extra field in
mtx_t definition or a GNU-only thread type (maybe not intended to be
exposed).


424: underspecification of tss_t

These issues are covered under DR 416.


449: What is the value of TSS_DTOR_ITERATIONS for implementations with no maximum?

The proposed committee response was to intentionally do not define a value of 
TSS_DTOR_ITERATIONS, so my view is align with PTHREAD_DESTRUCTOR_ITERATIONS 
should be suffice (even though various architectures redefine 
_POSIX_THREAD_DESTRUCTOR_ITERATIONS to same value in local_lim.h).


469: lock ownership vs. thread termination

Defined as future.  However from last committee discussion it seems that
they are aiming for similar POSIX description.  So I am not sure if there
is any impending stardard definition to use underlying POSIX implementation.


470: mtx_trylock should be allowed to fail spuriously

It align with POSIX and thus with __pthread_mutex_trylock which uses a 
atomic_compare_and_exchange_bool_acq (similar to atomic_compare_exchange_weak).


479: unclear specification of mtx_trylock on non-recursive muteness

Defined as future. I am not aware of any issue trying to use POSIX
in underlying implementation.


480: cnd_wait and cnd_timewait should allow spurious wake-ups

Define as open, although my understanding is C11 definitions from last proposed
technical corrigendum may now allow spurious wakes ups (from the words "until
it is unblocked due to an unspecified reason"). So I think implementing
cnd_{timed}wait as pthread_cond_{timed}wait wrapper should be still valid.


493: Mutex Initialization Underspecified

Define as open and the proposed committee response follows:

  - Problems with mtx_init(): I think the only applicable point to curent
    patch is the last one:

    7. thrd_error shall be returned by mtx_init() when passed a NULL pointer.

    Which currently leads to a segfault (since it is based on pthread_mutex_init).
    It is simple to align current patch proposal to this specification.

  - Problems with mtx_destroy(): wording aligns with POSIX definition, so
    implementing mtx_destroy as a pthread_mutex_destroy wrapper should be ok.

  - Other Problems: wording aligns with POSIX definition as well, so I think
    there is no requirement to change for current patch.
Torvald Riegel March 27, 2017, 9:25 a.m. UTC | #6
On Tue, 2017-03-21 at 16:49 +0000, Joseph Myers wrote:
> On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

> 

> > > Did you review those C11 DRs related to threads to make sure the 

> > > implementation behaves properly regarding issues raised in those DRs (and 

> > > that there are tests of this if applicable)?

> > > 

> > > Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

> > > 

> > 

> > Thanks for the link, I will check this out. Do you know off the top of your 

> > head which ones would be applicable? 

> 

> It appears to be: 405 414 416 424 449 469 470 479 480 493.

> 


Here's my take on these:

405, 414:
Some of these DRs affect implementations, but there's nothing
contentious in these I believe.

416, 424, 449:
I haven't looked at these before and can't comment on them currently.

469:
What I proposed should be clarified by C matches our implementation.
The committee seems to agree.
POSIX makes stronger requirements on the implementation, which we do not
implement (and what I do not intend to implement).  I have arguend in a
POSIX bug that POSIX should not make these stronger requirements (sorry,
don't have the bug number handy).

470:
C11's requirements on implementations are weaker than what POSIX
requires. Arguably, POSIX should require what C11 requires.
We need to check the lowlevellock implementation, in particular on archs
that use LL/SC (I'm not aware of an arch with a true CAS instruction
that can fail spuriously).
Our generic lowlevellock implementation on master still uses the
old-style atomics (ie, atomic_compare_and_exchange_bool_acq); if we move
to C11 atomics, C11 (with DR 470's proposed corrigendum applied) would
require our atomic_compare_exchange_weak_acquire, whereas current POSIX
would require the strong variant of the CAS.
Thus, what we do is okay in terms of C11.
I don't recall right now whether we have an open Austin Group bug about
POSIX allowing a spurious failure; I believe we have, or we should,
because this is also related to what memory order is required for lock
acquisitions (generally, not just trylock).
(Stefan, that's why I'm CC'ing you too, this is relevant for
pthread_spin_trylock too, but I can't remember right now whether we
already discussed this or not.)


479:
Our implementation is correct.
What C++ requires from a program, and what the committee agrees C11
should require, is stronger than what POSIX requires.  This means that
the C11 implementation of trylock would not have to conservatively abort
transactions in the lock elision implementation for the non-recursive
mutexes.

480:
A specification bug.  Committee agrees with what we implement.


493:
The intent of the standard as stated by the committee in this DR seems
to be compatible with what glibc implements and could implement in the
foreseeable future.

Martin,
please correct anything I got wrong.  Also, I assume you are tracking
these DRs, so please give us a heads up if the committee should change
its mind in a way that conflicts with what we implement.
Torvald Riegel March 27, 2017, 9:34 a.m. UTC | #7
On Tue, 2017-03-21 at 17:09 -0300, Adhemerval Zanella wrote:
> 405: The mutex specification

> 

> "The C11 specification of mutexes is missing the total order over all the

>  calls on a particular mutex."

> 

> My understanding is current implementation with POSIX primitives should be

> enough to guarantee C11 total order over mutex (which is currently based on

> acquire/release semanthics).


Yes.

> 414: Typos in 6.27 Threads <threads.h> 

> 

> The typos does clarify one definition of mtx_plain and raises a questioning: 

> how should we proceed if mtx_timedlock is used along with a mtx_plain mutex? 


If C11 requires that mtx_timedlock is supplied a mutex that supports
timeout.  If the program violates this requirement, it is undefined
behavior, and glibc is allowed to just acquire with a timeout.

> 469: lock ownership vs. thread termination


See my other response to this thread.

> 

> 470: mtx_trylock should be allowed to fail spuriously

> 

> It align with POSIX and thus with __pthread_mutex_trylock which uses a 

> atomic_compare_and_exchange_bool_acq (similar to atomic_compare_exchange_weak).


That's not quite true.  See my other response.

> 479: unclear specification of mtx_trylock on non-recursive muteness


> 480: cnd_wait and cnd_timewait should allow spurious wake-ups


See my other response to this thread.
Martin Sebor March 27, 2017, 4:10 p.m. UTC | #8
On 03/27/2017 03:25 AM, Torvald Riegel wrote:
> On Tue, 2017-03-21 at 16:49 +0000, Joseph Myers wrote:

>> On Tue, 21 Mar 2017, Adhemerval Zanella wrote:

>>

>>>> Did you review those C11 DRs related to threads to make sure the

>>>> implementation behaves properly regarding issues raised in those DRs (and

>>>> that there are tests of this if applicable)?

>>>>

>>>> Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm

>>>>

>>>

>>> Thanks for the link, I will check this out. Do you know off the top of your

>>> head which ones would be applicable?

>>

>> It appears to be: 405 414 416 424 449 469 470 479 480 493.

>>

>

> Here's my take on these:


I've reviewed the DRs and your comments below.  I agree with
your view and just for clarity provide some additional comments
of my own.

>

> 405, 414:

> Some of these DRs affect implementations, but there's nothing

> contentious in these I believe.


Agreed.

>

> 416, 424, 449:

> I haven't looked at these before and can't comment on them currently.


The 416 corrignedum was accepted.  424 was rolled into 416, and
449 resulted in no changes.

>

> 469:

> What I proposed should be clarified by C matches our implementation.

> The committee seems to agree.

> POSIX makes stronger requirements on the implementation, which we do not

> implement (and what I do not intend to implement).  I have arguend in a

> POSIX bug that POSIX should not make these stronger requirements (sorry,

> don't have the bug number handy).


This issue was closed with the expectation that it will be resolved
in C2X.  Unless someone else steps up it will likely mean that you
(or we) will have to propose the changes you/we would like to see.

> 470:

> C11's requirements on implementations are weaker than what POSIX

> requires. Arguably, POSIX should require what C11 requires.

> We need to check the lowlevellock implementation, in particular on archs

> that use LL/SC (I'm not aware of an arch with a true CAS instruction

> that can fail spuriously).

> Our generic lowlevellock implementation on master still uses the

> old-style atomics (ie, atomic_compare_and_exchange_bool_acq); if we move

> to C11 atomics, C11 (with DR 470's proposed corrigendum applied) would

> require our atomic_compare_exchange_weak_acquire, whereas current POSIX

> would require the strong variant of the CAS.

> Thus, what we do is okay in terms of C11.

> I don't recall right now whether we have an open Austin Group bug about

> POSIX allowing a spurious failure; I believe we have, or we should,

> because this is also related to what memory order is required for lock

> acquisitions (generally, not just trylock).

> (Stefan, that's why I'm CC'ing you too, this is relevant for

> pthread_spin_trylock too, but I can't remember right now whether we

> already discussed this or not.)

>


The resolution for the DR was accepted and the DR is closed.
The change will be in the 2017 C11 TC.  If other changes are
required here a new issue will need to be submitted that will
be considered for C2X.

>

> 479:

> Our implementation is correct.

> What C++ requires from a program, and what the committee agrees C11

> should require, is stronger than what POSIX requires.  This means that

> the C11 implementation of trylock would not have to conservatively abort

> transactions in the lock elision implementation for the non-recursive

> mutexes.


Here it sounds like as the authors of the DR we're on the hook
for providing wording that the rest of WG14 agrees with.  This
too will be considered for C2X.

>

> 480:

> A specification bug.  Committee agrees with what we implement.

>

>

> 493:

> The intent of the standard as stated by the committee in this DR seems

> to be compatible with what glibc implements and could implement in the

> foreseeable future.


This DR is still Open and needs a proposed resolution.

>

> Martin,

> please correct anything I got wrong.  Also, I assume you are tracking

> these DRs, so please give us a heads up if the committee should change

> its mind in a way that conflicts with what we implement.


Most of the DRs on the list are in a Closed state with (presumably)
clear resolutions.  As I understand the C11 schedule, the outstanding
ones that are still Open or labeled Future will not be fixed in
the final corrigendum that's expected to be finalized this year.
After 2017 there will not be any further C11 corrigenda.  C2X will
the next work item on WG14's list.

There's been a lot of talk over the last few WG14 meetings about
the whole threads section needing an overhaul.  I don't know if
anyone is actually working on it but if it were to happen (for
C2X) there is some risk that an implementation coded to the C11
spec not conforming to the cleaned up and improved C2X spec.

Martin

PS Besides the issues list there are at least two other documents
that could offer additional insight or guidance here.  The C2X
charter that outlines the guiding principles for C2X development:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2021.htm

In addition, the WG14 Convener maintains a standing document where
he tracks a subset of issues that have come up over the years and
that he wants WG14 to review for C2X.  This is not a complete list,
just some select items.

http://www.open-std.org/JTC1/SC22/WG14/www/docs/n2087.htm
Torvald Riegel March 28, 2017, 8:08 a.m. UTC | #9
On Mon, 2017-03-27 at 10:10 -0600, Martin Sebor wrote:
> I've reviewed the DRs and your comments below.  I agree with

> your view and just for clarity provide some additional comments

> of my own.


Thanks!

> There's been a lot of talk over the last few WG14 meetings about

> the whole threads section needing an overhaul.  I don't know if

> anyone is actually working on it but if it were to happen (for

> C2X) there is some risk that an implementation coded to the C11

> spec not conforming to the cleaned up and improved C2X spec.


I'd hope that they wouldn't deviate from what C++ specifies.  I'm
monitoring C++ changes, including whether anything would result in
required changes for glibc.  IOW, if C doesn't deviate from C++, we
shouldn't need additional changes just for C.
Adhemerval Zanella Netto March 31, 2017, 1:39 p.m. UTC | #10
On 28/03/2017 05:08, Torvald Riegel wrote:
> On Mon, 2017-03-27 at 10:10 -0600, Martin Sebor wrote:

>> I've reviewed the DRs and your comments below.  I agree with

>> your view and just for clarity provide some additional comments

>> of my own.

> 

> Thanks!

> 

>> There's been a lot of talk over the last few WG14 meetings about

>> the whole threads section needing an overhaul.  I don't know if

>> anyone is actually working on it but if it were to happen (for

>> C2X) there is some risk that an implementation coded to the C11

>> spec not conforming to the cleaned up and improved C2X spec.

> 

> I'd hope that they wouldn't deviate from what C++ specifies.  I'm

> monitoring C++ changes, including whether anything would result in

> required changes for glibc.  IOW, if C doesn't deviate from C++, we

> shouldn't need additional changes just for C.

> 


Thanks for both extensive inputs and discussion.  From the comments I 
see that a current C11 thread based on POSIX could be still be feasible,
however I am not sure if we should prevent its implementation based on 
the C2X possible different spec.

In any way, I see that the still pending DR493 should not pose any
implementation issues (we can work out on the wrapper if any other
requirement is posed).

So I would like the input from the community whether implementing C11
in GLIBC is desirable and if current approach based is most correct
one.
Torvald Riegel April 6, 2017, 11:05 a.m. UTC | #11
On Fri, 2017-03-31 at 10:39 -0300, Adhemerval Zanella wrote:
> 

> On 28/03/2017 05:08, Torvald Riegel wrote:

> > On Mon, 2017-03-27 at 10:10 -0600, Martin Sebor wrote:

> >> I've reviewed the DRs and your comments below.  I agree with

> >> your view and just for clarity provide some additional comments

> >> of my own.

> > 

> > Thanks!

> > 

> >> There's been a lot of talk over the last few WG14 meetings about

> >> the whole threads section needing an overhaul.  I don't know if

> >> anyone is actually working on it but if it were to happen (for

> >> C2X) there is some risk that an implementation coded to the C11

> >> spec not conforming to the cleaned up and improved C2X spec.

> > 

> > I'd hope that they wouldn't deviate from what C++ specifies.  I'm

> > monitoring C++ changes, including whether anything would result in

> > required changes for glibc.  IOW, if C doesn't deviate from C++, we

> > shouldn't need additional changes just for C.

> > 

> 

> Thanks for both extensive inputs and discussion.  From the comments I 

> see that a current C11 thread based on POSIX could be still be feasible,

> however I am not sure if we should prevent its implementation based on 

> the C2X possible different spec.

> 

> In any way, I see that the still pending DR493 should not pose any

> implementation issues (we can work out on the wrapper if any other

> requirement is posed).

> 

> So I would like the input from the community whether implementing C11

> in GLIBC is desirable and if current approach based is most correct

> one.


I think it is desirable, and it's probably about time that we have
something.  I'm not aware of anything that would be a huge problem.  C11
is in several cases much closer to what we implement than POSIX.

I still think it may have been nice to have smaller data structure sizes
for things like mutexes; however, we don't have consensus in the
community to shrink them, and we don't have the resources I believe to
really investigate this.

I haven't looked at the TLS issues, or at how you organize headers and
such.  I also haven't yet reviewed all of your patches.  But unless
somebody complains, IMHO we should just go forward with what you have.
Supporting the C11 threading support functions in the next release would
be nice.
Adhemerval Zanella Netto April 6, 2017, 2:29 p.m. UTC | #12
On 06/04/2017 08:05, Torvald Riegel wrote:
> On Fri, 2017-03-31 at 10:39 -0300, Adhemerval Zanella wrote:

>>

>> On 28/03/2017 05:08, Torvald Riegel wrote:

>>> On Mon, 2017-03-27 at 10:10 -0600, Martin Sebor wrote:

>>>> I've reviewed the DRs and your comments below.  I agree with

>>>> your view and just for clarity provide some additional comments

>>>> of my own.

>>>

>>> Thanks!

>>>

>>>> There's been a lot of talk over the last few WG14 meetings about

>>>> the whole threads section needing an overhaul.  I don't know if

>>>> anyone is actually working on it but if it were to happen (for

>>>> C2X) there is some risk that an implementation coded to the C11

>>>> spec not conforming to the cleaned up and improved C2X spec.

>>>

>>> I'd hope that they wouldn't deviate from what C++ specifies.  I'm

>>> monitoring C++ changes, including whether anything would result in

>>> required changes for glibc.  IOW, if C doesn't deviate from C++, we

>>> shouldn't need additional changes just for C.

>>>

>>

>> Thanks for both extensive inputs and discussion.  From the comments I 

>> see that a current C11 thread based on POSIX could be still be feasible,

>> however I am not sure if we should prevent its implementation based on 

>> the C2X possible different spec.

>>

>> In any way, I see that the still pending DR493 should not pose any

>> implementation issues (we can work out on the wrapper if any other

>> requirement is posed).

>>

>> So I would like the input from the community whether implementing C11

>> in GLIBC is desirable and if current approach based is most correct

>> one.

> 

> I think it is desirable, and it's probably about time that we have

> something.  I'm not aware of anything that would be a huge problem.  C11

> is in several cases much closer to what we implement than POSIX.

> 

> I still think it may have been nice to have smaller data structure sizes

> for things like mutexes; however, we don't have consensus in the

> community to shrink them, and we don't have the resources I believe to

> really investigate this.


Indeed, however this would add some complication to use the POSIX internal
implementation directly.  We would need to create a common internal function
that would be called by POSIX and C11 wrappers, I will check this out in my
next rebase.

> 

> I haven't looked at the TLS issues, or at how you organize headers and

> such.  I also haven't yet reviewed all of your patches.  But unless

> somebody complains, IMHO we should just go forward with what you have.

> Supporting the C11 threading support functions in the next release would

> be nice.


Now with the removal of the ancient macro CALL_THREAD_FCT [1] and the
consolidation of pthreadtypes.h in a different patchset [2] [3], I will
rebase the changes and send a new patchset.  The consolidation of
pthreadtypes.h is mostly a mechanical change without expected code changes,
so I think it should be safer to push i

[1] https://sourceware.org/ml/libc-alpha/2017-04/msg00057.html
[2] https://sourceware.org/ml/libc-alpha/2017-04/msg00018.html
[3] https://sourceware.org/ml/libc-alpha/2017-04/msg00019.html