Re: [PATCH v3 31/37] nptl: Move internal symbol __mutex_aconf into libc

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 16/03/2021 14:31, Florian Weimer via Libc-alpha wrote:
> This is in preparation of moving the mutex code into libc.
> 
> __pthread_tunables_init is now called via __libc_early_init.
> For non-NPTL targets, a stub version is provided.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  nptl/Makefile                               |  2 +-
>  nptl/Versions                               |  1 +
>  nptl/nptl-init.c                            |  4 ---
>  nptl/pthread_mutex_conf.c                   |  1 +
>  sysdeps/nptl/pthread_early_init.h           | 33 +++++++++++++++++++++
>  {nptl => sysdeps/nptl}/pthread_mutex_conf.h |  9 +++++-
>  sysdeps/pthread/pthread_mutex_conf.h        | 28 +++++++++++++++++
>  7 files changed, 72 insertions(+), 6 deletions(-)
>  create mode 100644 sysdeps/nptl/pthread_early_init.h
>  rename {nptl => sysdeps/nptl}/pthread_mutex_conf.h (86%)
>  create mode 100644 sysdeps/pthread/pthread_mutex_conf.h
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 353cbf8911..3ae0e1e8c8 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -82,6 +82,7 @@ routines = \
>    pthread_key_create \
>    pthread_key_delete \
>    pthread_keys \
> +  pthread_mutex_conf \
>    pthread_mutex_consistent \
>    pthread_once \
>    pthread_rwlock_rdlock \
> @@ -170,7 +171,6 @@ libpthread-routines = \
>    pthread_kill \
>    pthread_kill_other_threads \
>    pthread_mutex_cond_lock \
> -  pthread_mutex_conf \
>    pthread_mutex_destroy \
>    pthread_mutex_getprioceiling \
>    pthread_mutex_init \

Ok.

> diff --git a/nptl/Versions b/nptl/Versions
> index 4dee682ed9..2b9508bd6f 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -136,6 +136,7 @@ libc {
>      __lll_lock_wait_private;
>      __lll_trylock_elision;
>      __lll_unlock_elision;
> +    __mutex_aconf;
>      __nptl_deallocate_tsd;
>      __nptl_nthreads;
>      __pthread_attr_copy;

Ok.

> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index 0c07b46a77..5c85f75cc1 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -269,10 +269,6 @@ __pthread_initialize_minimal_internal (void)
>    __libc_multiple_threads_ptr =
>  #endif
>      __libc_pthread_init (__reclaim_stacks, ptr_pthread_functions);
> -
> -#if HAVE_TUNABLES
> -  __pthread_tunables_init ();
> -#endif
>  }
>  strong_alias (__pthread_initialize_minimal_internal,
>  	      __pthread_initialize_minimal)

Ok.

> diff --git a/nptl/pthread_mutex_conf.c b/nptl/pthread_mutex_conf.c
> index e6532ba1d7..e6235dea47 100644
> --- a/nptl/pthread_mutex_conf.c
> +++ b/nptl/pthread_mutex_conf.c
> @@ -30,6 +30,7 @@ struct mutex_config __mutex_aconf =
>    calling into kernel to block.  */
>    .spin_count = DEFAULT_ADAPTIVE_COUNT,
>  };
> +libc_hidden_data_def (__mutex_aconf)
>  
>  static void
>  TUNABLE_CALLBACK (set_mutex_spin_count) (tunable_val_t *valp)

Ok.

> diff --git a/sysdeps/nptl/pthread_early_init.h b/sysdeps/nptl/pthread_early_init.h
> new file mode 100644
> index 0000000000..69e700dae1
> --- /dev/null
> +++ b/sysdeps/nptl/pthread_early_init.h
> @@ -0,0 +1,33 @@
> +/* pthread initialization called from __libc_early_init.  NPTL version.
> +   Copyright (C) 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/>.  */
> +
> +#ifndef _PTHREAD_EARLY_INIT_H
> +#define _PTHREAD_EARLY_INIT_H 1
> +
> +#include <nptl/pthreadP.h>
> +#include <pthread_mutex_conf.h>
> +
> +static inline void
> +__pthread_early_init (void)
> +{
> +#if HAVE_TUNABLES
> +  __pthread_tunables_init ();
> +#endif
> +}
> +
> +#endif /* _PTHREAD_EARLY_INIT_H */

Double space before '*/'.

> diff --git a/nptl/pthread_mutex_conf.h b/sysdeps/nptl/pthread_mutex_conf.h
> similarity index 86%
> rename from nptl/pthread_mutex_conf.h
> rename to sysdeps/nptl/pthread_mutex_conf.h
> index 14e135a539..9dba4bfa6d 100644
> --- a/nptl/pthread_mutex_conf.h
> +++ b/sysdeps/nptl/pthread_mutex_conf.h
> @@ -26,9 +26,16 @@ struct mutex_config
>    int spin_count;
>  };
>  
> -extern struct mutex_config __mutex_aconf attribute_hidden;
> +extern struct mutex_config __mutex_aconf;
> +libc_hidden_proto (__mutex_aconf)
>  
>  extern void __pthread_tunables_init (void) attribute_hidden;
> +#else
> +static inline void
> +__pthread_tunables_init (void)
> +{
> +  /* No tunables to initialize.  */
> +}
>  #endif
>  
>  #endif

Ok.

> diff --git a/sysdeps/pthread/pthread_mutex_conf.h b/sysdeps/pthread/pthread_mutex_conf.h
> new file mode 100644
> index 0000000000..c999c980f7
> --- /dev/null
> +++ b/sysdeps/pthread/pthread_mutex_conf.h
> @@ -0,0 +1,28 @@
> +/* Pthread mutex tunable parameters.  Generic version.
> +   Copyright (C) 2018-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/>.  */
> +
> +#ifndef _PTHREAD_MUTEX_CONF_H
> +#define _PTHREAD_MUTEX_CONF_H 1
> +
> +static inline void
> +__pthread_tunables_init (void)
> +{
> +  /* The generic version does not have any tunables to init.  */
> +}
> +
> +#endif
> 
Ok.